Closed Bug 462948 Opened 13 years ago Closed 13 years ago
lint warnings for source files that include keythi
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.