Closed Bug 462948 Opened 13 years ago Closed 13 years ago

lint warnings for source files that include keythi.h

Categories

(NSS :: Libraries, defect)

3.11
Sun
Solaris
defect
Not set
minor

Tracking

(Not tracked)

RESOLVED FIXED
3.11.10

People

(Reporter: nelson, Assigned: nelson)

Details

Attachments

(1 file)

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)
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.
Summary: compiler warnings for source files that include keythi.h → lint warnings for source files that include keythi.h
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.
I believe the old type and the new one are both effectively 
const unsigned int
Attachment #346197 - Flags: review?(julien.pierre.boogz)
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.
Attachment #346197 - Flags: review?(julien.pierre.boogz) → review+
lib/cryptohi/keythi.h; new revision: 1.11; previous revision: 1.10
Status: NEW → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.