808218 - Update more NSS declarations to have const parameters

Status: RESOLVED FIXED

(NSS :: Libraries, defect)

Description

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

Attachment: Update more NSS declarations to have const parameters

The changes that implement the B2G signature verification logic depend on these changes that change some parameters of some functions "X*" parameters to "const X*" parameters. We could also just use casts in the code, but I'd rather clean these declarations up if/when we can.

Comment 1

[Robert Relyea]

Comment on attachment 677915 [details] [diff] [review]
Update more NSS declarations to have const parameters

r+ rrelyea

Comment 2

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

Attachment: Update more NSS declarations to have const parameters [v2]

This version adds PK11_Digest* and also fixes some const-related warnings. (Some parts of the previous version of the patch removed const-related warnings from the application, but added them into NSS.)

Perhaps we should update the PKCS#11 headers to be const-correct too. That would eliminate most of the need to cast. Note that I used the PKCS#11 type (CK_BYTE_PTR) instead of (PRUint8*) to hint that this cast is needed because the PKCS#11 declarations are wrong.

Comment 3

[Robert Relyea]

Comment on attachment 681717 [details] [diff] [review]
Update more NSS declarations to have const parameters [v2]

r-

PK11_DigestFinal isn't a const. data is actually returned. The rest look fine...

Comment 4

[Robert Relyea]

> Perhaps we should update the PKCS#11 headers to be const-correct too.

I'm of two minds about this. It's a good idea in general, but unfortunately a lot of things depend on these headers (stuff under ckfw, and things that aren't even in the NSS tree). We would have to coordinate such a change.

bob

Comment 5

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

Attachment: Update more NSS declarations to have const parameters [v3]

Thanks for catching that, Bob. I don't know what I was thinking.

Comment 6

[Robert Relyea]

Comment on attachment 682486 [details] [diff] [review]
Update more NSS declarations to have const parameters [v3]

r+ on marches the progress of cleaning up const;).

bob

Comment 7

[Lucas Adamski [:ladamski]]

Necessary for app signing (bug 772365).

Comment 8

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

Attachment: Update constness of PK11_Sign and PK11_Verify*

Comment 9

[Wan-Teh Chang]

Comment on attachment 682486 [details] [diff] [review]
Update more NSS declarations to have const parameters [v3]

Review of attachment 682486 [details] [diff] [review]:
-----------------------------------------------------------------

r=wtc.

Comment 10

[Wan-Teh Chang]

Comment on attachment 684282 [details] [diff] [review]
Update constness of PK11_Sign and PK11_Verify*

Review of attachment 684282 [details] [diff] [review]:
-----------------------------------------------------------------

r=wtc.

Note that adding 'const' to 'SECItem *' input arguments is a
cosmetic change because the real constness problem is with
the |data| fields of those SECItem structures.

Comment 11

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

Checking in security/nss/lib/nss/utilwrap.c;
/cvsroot/mozilla/security/nss/lib/nss/utilwrap.c,v  <--  utilwrap.c
new revision: 1.8; previous revision: 1.7
done
Checking in security/nss/lib/pk11wrap/pk11cxt.c;
/cvsroot/mozilla/security/nss/lib/pk11wrap/pk11cxt.c,v  <--  pk11cxt.c
new revision: 1.10; previous revision: 1.9
done
Checking in security/nss/lib/pk11wrap/pk11obj.c;
/cvsroot/mozilla/security/nss/lib/pk11wrap/pk11obj.c,v  <--  pk11obj.c
new revision: 1.29; previous revision: 1.28
done
Checking in security/nss/lib/pk11wrap/pk11pub.h;
/cvsroot/mozilla/security/nss/lib/pk11wrap/pk11pub.h,v  <--  pk11pub.h
new revision: 1.42; previous revision: 1.41
done
Checking in security/nss/lib/pkcs7/p7decode.c;
/cvsroot/mozilla/security/nss/lib/pkcs7/p7decode.c,v  <--  p7decode.c
new revision: 1.30; previous revision: 1.29
done
Checking in security/nss/lib/pkcs7/secpkcs7.h;
/cvsroot/mozilla/security/nss/lib/pkcs7/secpkcs7.h,v  <--  secpkcs7.h
new revision: 1.10; previous revision: 1.9
done
Checking in security/nss/lib/util/base64.h;
/cvsroot/mozilla/security/nss/lib/util/base64.h,v  <--  base64.h
new revision: 1.5; previous revision: 1.4
done
Checking in security/nss/lib/util/nssb64d.c;
/cvsroot/mozilla/security/nss/lib/util/nssb64d.c,v  <--  nssb64d.c
new revision: 1.10; previous revision: 1.9
done

Comment 12

[Mike Hommey [:glandium]]

(In reply to Brian Smith (:bsmith) from comment #0)
> Created attachment 677915 [details] [diff] [review]
> Update more NSS declarations to have const parameters
> 
> The changes that implement the B2G signature verification logic depend on
> these changes that change some parameters of some functions "X*" parameters
> to "const X*" parameters. We could also just use casts in the code, but I'd
> rather clean these declarations up if/when we can.

That's really not good enough a reason to introduce such changes in NSS, especially considering the implications that has, more so when the patch in question is uplifted up to beta. Cf. comments in bug 818717.

Comment 13

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

Fixed in bug 818717 for mozilla-aurora and mozilla-beta.