943297 - variable ‘mode’ set but not used in ‘jar_listtar’

Status: RESOLVED DUPLICATE of bug 1182667

(NSS :: Libraries, defect)

Description

[Maxim Philippov]

Attachment: proposed patch

User Agent: Mozilla/5.0 (X11; Linux x86_64; rv:28.0) Gecko/20100101 Firefox/28.0 (Beta/Release)
Build ID: 20131121171534

Steps to reproduce:

Compiled against revision 157402:53d55d2d0a25


Actual results:

Compiler warned about set but not used variables in security/nss/lib/jar/jarfile.c

gcc -o /home/maxim/projects/mozilla/obj-x86_64-unknown-linux-gnu/security/nss/lib/jar/jarfile.o -c -Os -gdwarf-2 -D_POSIX_SOURCE -D_BSD_SOURCE -D_XOPEN_SOURCE -fPIC -DLINUX2_1  -Wall -Werror-implicit-function-declaration -Wno-switch -pipe -DLINUX -Dlinux -DHAVE_STRERROR -DXP_UNIX -DMOZILLA_CLIENT=1 -UDEBUG -DNDEBUG -D_REENTRANT -DNSS_ENABLE_ECC -DUSE_UTIL_DIRECTLY -DNO_NSPR_10_SUPPORT -DNSS_X86_OR_X64 -DNSS_X64 -I/home/maxim/projects/mozilla/obj-x86_64-unknown-linux-gnu/dist/include/nspr -I/home/maxim/projects/mozilla/obj-x86_64-unknown-linux-gnu/dist/include/nspr -I/home/maxim/projects/mozilla/obj-x86_64-unknown-linux-gnu/dist/include/nss -I/home/maxim/projects/mozilla/obj-x86_64-unknown-linux-gnu/dist/private/nss  jarfile.c
jarfile.c: In function ‘jar_listtar’:
jarfile.c:824:12: warning: variable ‘when’ set but not used [-Wunused-but-set-variable]
     time_t when;
            ^
jarfile.c:823:14: warning: variable ‘mode’ set but not used [-Wunused-but-set-variable]
     long sz, mode;
              ^

There is no place for `when' in JAR_Physical structure, but `mode' may be stored and used on Unix and BeOS systems.
In the attached patch `when' variable is removed and `mode' is stored in JAR_Physical structure. Maybe `when' should be used somehow, but I don't see how.

Comment 1

[:Cykesiopka]

Attachment: bug943297_fix-warn-mode-jar_listtar_by-Maxim-Philippov_v1.patch

This is Maxim's patch, but against the NSS repo.

Comment 2

[Kai Engert (:KaiE:)]

Assigning to phy->mode is a functional change, and it isn't obvious if it could have undesired effects.

Unless someone tests what happens exactly here, I'd rather keep the change, to ensure we don't modify the intended behaviour.

If you want to silence the message, I'd rather wrap the assignment to mode in a comment, and state in the comment that it's unclear why this wasn't used.

Comment 3

[:Cykesiopka]

(In reply to Kai Engert (:kaie) from comment #2)
> Assigning to phy->mode is a functional change, and it isn't obvious if it
> could have undesired effects.
> 
> Unless someone tests what happens exactly here, I'd rather keep the change,
> to ensure we don't modify the intended behaviour.
> 
> If you want to silence the message, I'd rather wrap the assignment to mode
> in a comment, and state in the comment that it's unclear why this wasn't
> used.

Thanks. Looks like the patch for Bug 1182667 fixes this warning as well:
https://hg.mozilla.org/projects/nss/rev/4355f55afeb2#l101.153

I'm cancelling review for now, and will make this bug a duplicate of Bug 1182667 if/when it lands with the relevant changes still included.

Comment 4

[:Cykesiopka]

(In reply to Cykesiopka from comment #3)
> I'm cancelling review for now, and will make this bug a duplicate of Bug
> 1182667 if/when it lands with the relevant changes still included.

The relevant change in Bug 1182667 has stuck for a while, so marking as dupe.