Closed
Bug 578861
Opened 14 years ago
Closed 12 years ago
CERT_CompareName should take const input parameters
Categories
(NSS :: Libraries, defect)
NSS
Libraries
Tracking
(Not tracked)
RESOLVED
FIXED
3.14
People
(Reporter: wtc, Assigned: KaiE)
Details
Attachments
(1 file, 3 obsolete files)
3.24 KB,
patch
|
wtc
:
review+
|
Details | Diff | Splinter Review |
The input parameters of CERT_CompareName should be declared with 'const'.
Assignee | ||
Comment 1•12 years ago
|
||
Attachment #662496 -
Flags: review?(wtc)
Comment 2•12 years ago
|
||
Comment on attachment 662496 [details] [diff] [review] patch v1 Review of attachment 662496 [details] [diff] [review]: ----------------------------------------------------------------- ::: mozilla/security/nss/lib/certdb/secname.c @@ +577,3 @@ > { > + const CERTRDN **ardns, *ardn; > + const CERTRDN **brdns, *brdn; Doesn't this need to be this?: CERTRDN const * const * ardns, const *ardn; CERTRDN const * const * brdns, const *brdn; Otherwise, you should get a const conversion warning when assigning ardn = *ardns below. CERT_CompareRDN should take const parameters, to avoid the same kinds of warnings. Similarly, CountArray's argument type should become void const * const *, and the cast in the call to CountArrays can become (void const * const *) instead of (void**).
Reporter | ||
Comment 3•12 years ago
|
||
Comment on attachment 662496 [details] [diff] [review] patch v1 Review of attachment 662496 [details] [diff] [review]: ----------------------------------------------------------------- r=wtc. ::: mozilla/security/nss/lib/certdb/secname.c @@ +577,3 @@ > { > + const CERTRDN **ardns, *ardn; > + const CERTRDN **brdns, *brdn; If we add more consts to the local variable declarations, we should declare only one variable per line, otherwise it's very confusing. const CERTRDN *const *ardns; const CERTRDN *ardn; const CERTRDN *const *brdns; const CERTRDN *brdn; But I am fine with your current patch. Please also change CERT_CompareRDN. Thanks.
Attachment #662496 -
Flags: review?(wtc) → review+
Assignee | ||
Comment 4•12 years ago
|
||
Attachment #662496 -
Attachment is obsolete: true
Attachment #662551 -
Flags: review?(wtc)
Assignee | ||
Comment 5•12 years ago
|
||
Attachment #662551 -
Attachment is obsolete: true
Attachment #662551 -
Flags: review?(wtc)
Attachment #662552 -
Flags: review?(wtc)
Assignee | ||
Comment 6•12 years ago
|
||
Attachment #662552 -
Attachment is obsolete: true
Attachment #662552 -
Flags: review?(wtc)
Attachment #662561 -
Flags: review?(wtc)
Reporter | ||
Comment 7•12 years ago
|
||
Comment on attachment 662561 [details] [diff] [review] v5 r=wtc. Thanks. After some testing and discussions with Kai, we decided to only add const to the input arguments of CERT_CompareName and CERT_CompareRDN. We are not adding const to the local variables because they are passed to the CountArray function, and we can't figure out a portable way to declare the input argument of CountArray so that we don't need any casts in the CountArray calls.
Attachment #662561 -
Flags: review?(wtc) → review+
Assignee | ||
Comment 8•12 years ago
|
||
Checking in cert.h; /cvsroot/mozilla/security/nss/lib/certdb/cert.h,v <-- cert.h new revision: 1.90; previous revision: 1.89 done Checking in secname.c; /cvsroot/mozilla/security/nss/lib/certdb/secname.c,v <-- secname.c new revision: 1.28; previous revision: 1.27 done
Assignee: wtc → kaie
Status: NEW → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Target Milestone: --- → 3.14
You need to log in
before you can comment on or make changes to this bug.
Description
•