Closed
Bug 462948
Opened 16 years ago
Closed 16 years ago
lint warnings for source files that include keythi.h
Categories
(NSS :: Libraries, defect)
Tracking
(Not tracked)
RESOLVED
FIXED
3.11.10
People
(Reporter: nelson, Assigned: nelson)
Details
Attachments
(1 file)
1.06 KB,
patch
|
julien.pierre
:
review+
|
Details | Diff | Splinter Review |
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•16 years ago
|
||
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.
Updated•16 years ago
|
Summary: compiler warnings for source files that include keythi.h → lint warnings for source files that include keythi.h
Comment 2•16 years ago
|
||
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.
Assignee | ||
Comment 3•16 years ago
|
||
I believe the old type and the new one are both effectively
const unsigned int
Attachment #346197 -
Flags: review?(julien.pierre.boogz)
Comment 4•16 years ago
|
||
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+
Assignee | ||
Comment 5•16 years ago
|
||
lib/cryptohi/keythi.h; new revision: 1.11; previous revision: 1.10
Status: NEW → RESOLVED
Closed: 16 years ago
Resolution: --- → FIXED
You need to log in
before you can comment on or make changes to this bug.
Description
•