462948 - lint warnings for source files that include keythi.h

Status: RESOLVED FIXED

(NSS :: Libraries, defect)

Description

[Nelson Bolyard (seldom reads bugmail)]

The NSS header file keythi.h defines a static const int value.
If the c source file that #includes "keythi.h" doesn't use that value,
a warning is given.  (CR 6492310)

The relevant lines of code are:

205 #define CachedAttribute(attribute,setbit) \
206 static const PRUint32 SECKEY_##attribute = 1 << setbit;
207 
208 /* bit flag definitions for staticflags */
209 #define SECKEY_Attributes_Cached 0x1    /* bit 0 states
210                                            whether attributes are cached */
211 CachedAttribute(CKA_PRIVATE,1) /* bit 1 is the value of CKA_PRIVATE */

This warning can be eliminated by eliminating lines 205-206, and replacing 
line 211 shown above with: 

    #define SECKEY_CKA_PRIVATE (1U << 1)

Comment 1

[Julien Pierre]

This is actually a lint warning. Not a single compiler gives a warning on this code, AFAIK. IMO, the code checked in is one perfectly valid way of defining a constant, and arguably a more modern one than using a macro (#define).

IMO, lint is being overly aggressive here. It should detect that this static is a const, and not complain. If this was a non-const static I would agree with the lint warning and that it should be taken out. Perhaps lint has a mode where this warning can be silenced.

Comment 2

[Julien Pierre]

Just to be clear, the reason why I prefer the current code and wrote it this way is that the constant is typed. When using #define, the constant is untyped. I am in favor of stronger typing, not weaker.

Comment 3

[Nelson Bolyard (seldom reads bugmail)]

Attachment: Patch v1 for NSS Trunk

I believe the old type and the new one are both effectively 
const unsigned int

Comment 4

[Julien Pierre]

Comment on attachment 346197 [details] [diff] [review]
Patch v1 for NSS Trunk

This is OK because the value in the macro is typed.

Both constructs are correct, but I would much prefer to keep the current code.

An unused constant is never an error, nor should it be a warning, whether that constant is declared as a static const or as a macro. I would like to know what reason lint has for telling us we should use the later construct.

I would rather close this one as a WONTFIX than see this go in. But I won't argue any further if you do. We have already spent far too much time on this than it deserves.

Comment 5

[Nelson Bolyard (seldom reads bugmail)]

lib/cryptohi/keythi.h; new revision: 1.11; previous revision: 1.10