288730 - compiler warnings in certutil

Status: RESOLVED FIXED

(NSS :: Tools, defect, P5)

Description

[Julien Pierre]

I found the following warnings in certutil.

One of them is about dead code - printKeyCB - which seems to be applicable to
RSA only. I think we should delete it.

The others are about unspecified case in a switch statement. I think that one
needs a closer examination. Any taker ?

gcc -o SunOS5.10_i86pc_gcc_64_DBG.OBJ/certutil.o -c -g -Wall -Wno-format -fPIC
-DSVR4 -DSYSV -D__svr4 -D__svr4__ -DSOLARIS -D_REENTRANT -DSOLARIS2_10
-D_SVID_GETTOD -m64 -DXP_UNIX -DNSPR20 -DDEBUG -UNDEBUG -DDEBUG_jp96085
-I/usr/dt/include -I/usr/openwin/include
-I../../../../dist/SunOS5.10_i86pc_gcc_64_DBG.OBJ/include 
-I../../../../dist/public/nss  -I../../../../dist/private/nss 
-I../../../../dist/public/dbm -I../../../../dist/public/seccmd  certutil.c
certutil.c:873: warning: 'printKeyCB' defined but not used
gcc -o SunOS5.10_i86pc_gcc_64_DBG.OBJ/keystuff.o -c -g -Wall -Wno-format -fPIC
-DSVR4 -DSYSV -D__svr4 -D__svr4__ -DSOLARIS -D_REENTRANT -DSOLARIS2_10
-D_SVID_GETTOD -m64 -DXP_UNIX -DNSPR20 -DDEBUG -UNDEBUG -DDEBUG_jp96085
-I/usr/dt/include -I/usr/openwin/include
-I../../../../dist/SunOS5.10_i86pc_gcc_64_DBG.OBJ/include 
-I../../../../dist/public/nss  -I../../../../dist/private/nss 
-I../../../../dist/public/dbm -I../../../../dist/public/seccmd  keystuff.c
keystuff.c: In function `CERTUTIL_GeneratePrivateKey':
keystuff.c:581: warning: enumeration value `nullKey' not handled in switch
keystuff.c:581: warning: enumeration value `fortezzaKey' not handled in switch
keystuff.c:581: warning: enumeration value `dhKey' not handled in switch
keystuff.c:581: warning: enumeration value `keaKey' not handled in switch
keystuff.c:581: warning: enumeration value `ecKey' not handled in switch

Comment 1

[Nelson Bolyard (seldom reads bugmail)]

This is another case of warnings run amok.  The code is perfectly valid c.
There's no reason to believe it has found anything but code working as intended.

We could silence it by adding a default label just before the end of 
the switch.  But I'd prefer to find the command line option that 
turns off these nonsense warnings, and add it to coreconf.

Comment 2

[Julien Pierre]

From http://gcc.gnu.org/onlinedocs/gcc/Warning-Options.html :

-Wswitch
    Warn whenever a switch statement has an index of enumerated type and lacks a case for one or more of the named codes of that enumeration. (The presence of a default label prevents this warning.) case labels outside the enumeration range also provoke warnings when this option is used. This warning is enabled by -Wall.
-Wswitch-default
    Warn whenever a switch statement does not have a default case.
-Wswitch-enum
    Warn whenever a switch statement has an index of enumerated type and lacks a case for one or more of the named codes of that enumeration. case labels outside the enumeration range also provoke warnings when this option is used.
-Wtrigraphs

Comment 3

[Nelson Bolyard (seldom reads bugmail)]

which setting says: don't warn me about missing enum cases in switches?

Comment 4

[Julien Pierre]

I believe -Wnoswitch . From the link :

You can request many specific warnings with options beginning `-W', for example -Wimplicit to request warnings on implicit declarations. Each of these specific warning options also has a negative form beginning `-Wno-' to turn off warnings; for example, -Wno-implicit. This manual lists only one of the two forms, whichever is not the default.

Comment 5

[Biswatosh Chakraborty]

(1)
-Wno-switch removed the warnings. 
The warnings were coming because -Wall was enabled and thus caused -Wswitch
to be enabled, which means that a switch statement has an index of enumerated type and lacks a case for one or more of the named codes of that enumeration. See Comment #2.

(2)
printKeyCB() present in certutil.c is not called by any other function and should probably be removed.

(3)
There was another warning about an unused variable 'i' in keystuff.c in the
function UpdateRNG(). 


My patch(to be attached in next few minutes) 

(a)adds -Wno-switch wherever gcc is used with -Wall. And, it covers
several .mk files in coreconf such as Linux.mk,SunOS5.mk,OpenBSD.mk,OS2.mk etc.

(b)removes the unused variable 'i' from UpdateRNG().

and

(c)removes printKeyCB() from certutil.c.

Comment 6

[Biswatosh Chakraborty]

Attachment: For review. Removes the warnings by adding -Wno-switch.

Please refer Comment #5 for the patch details.

Comment 7

[Julien Pierre]

Comment on attachment 280699 [details] [diff] [review]
For review.  Removes the warnings by adding -Wno-switch.

Looks good.

Comment 8

[Biswatosh Chakraborty]

Checking in  .mk files of coreconf to the trunk...



Checking in BSD_OS.mk;
/cvsroot/mozilla/security/coreconf/BSD_OS.mk,v  <--  BSD_OS.mk
new revision: 1.5; previous revision: 1.4
done

Checking in BeOS.mk;
/cvsroot/mozilla/security/coreconf/BeOS.mk,v  <--  BeOS.mk
new revision: 1.3; previous revision: 1.2
done

Checking in FreeBSD.mk;
/cvsroot/mozilla/security/coreconf/FreeBSD.mk,v  <--  FreeBSD.mk
new revision: 1.10; previous revision: 1.9
done

Checking in IRIX.mk;
/cvsroot/mozilla/security/coreconf/IRIX.mk,v  <--  IRIX.mk
new revision: 1.7; previous revision: 1.6
done

Checking in Linux.mk;
/cvsroot/mozilla/security/coreconf/Linux.mk,v  <--  Linux.mk
new revision: 1.31; previous revision: 1.30
done

Checking in NCR3.0.mk;
/cvsroot/mozilla/security/coreconf/NCR3.0.mk,v  <--  NCR3.0.mk
new revision: 1.5; previous revision: 1.4
done

Checking in NetBSD.mk;
/cvsroot/mozilla/security/coreconf/NetBSD.mk,v  <--  NetBSD.mk
new revision: 1.6; previous revision: 1.5
done
Checking in OS2.mk;
/cvsroot/mozilla/security/coreconf/OS2.mk,v  <--  OS2.mk
new revision: 1.28; previous revision: 1.27
done

Checking in OpenBSD.mk;
/cvsroot/mozilla/security/coreconf/OpenBSD.mk,v  <--  OpenBSD.mk
new revision: 1.6; previous revision: 1.5
done
Checking in ReliantUNIX.mk;
/cvsroot/mozilla/security/coreconf/ReliantUNIX.mk,v  <--  ReliantUNIX.mk
new revision: 1.6; previous revision: 1.5
done

Checking in SunOS4.1.3_U1.mk;
/cvsroot/mozilla/security/coreconf/SunOS4.1.3_U1.mk,v  <--  SunOS4.1.3_U1.mk
new revision: 1.4; previous revision: 1.3
done
Checking in SunOS5.4_i86pc.mk;
/cvsroot/mozilla/security/coreconf/SunOS5.4_i86pc.mk,v  <--  SunOS5.4_i86pc.mk
new revision: 1.4; previous revision: 1.3
done

Checking in SunOS5.mk;
/cvsroot/mozilla/security/coreconf/SunOS5.mk,v  <--  SunOS5.mk
new revision: 1.25; previous revision: 1.24
done

Comment 9

[Biswatosh Chakraborty]

Checking in certutil.c;
/cvsroot/mozilla/security/nss/cmd/certutil/certutil.c,v  <--  certutil.c
new revision: 1.122; previous revision: 1.121
done

Checking in keystuff.c;
/cvsroot/mozilla/security/nss/cmd/certutil/keystuff.c,v  <--  keystuff.c
new revision: 1.18; previous revision: 1.17
done