578861 - CERT_CompareName should take const input parameters

Status: RESOLVED FIXED

(NSS :: Libraries, defect)

Description

[Wan-Teh Chang]

The input parameters of CERT_CompareName should be declared
with 'const'.

Comment 1

[Kai Engert (:KaiE:)]

Attachment: patch v1

Comment 2

[Brian Smith (:briansmith, :bsmith, use NEEDINFO?)]

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**).

Comment 3

[Wan-Teh Chang]

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.

Comment 4

[Kai Engert (:KaiE:)]

Attachment: patch v2

Comment 5

[Kai Engert (:KaiE:)]

Attachment: v3

Comment 6

[Kai Engert (:KaiE:)]

Attachment: v5

Comment 7

[Wan-Teh Chang]

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.

Comment 8

[Kai Engert (:KaiE:)]

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