286313 - pk11_getKeyFromList can call PORT_Alloc instead of PORT_ZAlloc

Status: RESOLVED FIXED

(NSS :: Libraries, defect)

Description

[Wan-Teh Chang]

pk11_getKeyFromList uses PORT_ZAlloc to allocate
a new PK11SymKey structure if it can't find a
PK11SymKey object from the freelist.

By code inspection, I found that all members of
the newly allocated PK11SymKey structure are
immediately assigned a value in either
pk11_getKeyFromList or PK11_CreateSymKey (the
only caller of pk11_getKeyFromList).

This means pk11_getKeyFromList can simply call
PORT_Alloc instead.

Comment 1

[Wan-Teh Chang]

Attachment: Proposed patch

Actually, one member of PK11SymKey is
not assigned a value after it is allocated:
data.type ('data' is a SECItem).  So this
patch also assigns that member to siBuffer
(0).  I suspect our code isn't using that
member anyway.

Comment 2

[Wan-Teh Chang]

I checked in the patch on the NSS trunk for NSS 3.10.

Checking in pk11skey.c;
/cvsroot/mozilla/security/nss/lib/pk11wrap/pk11skey.c,v  <--  pk11skey.c
new revision: 1.92; previous revision: 1.91
done

Comment 3

[Nelson Bolyard (seldom reads bugmail)]

Comment on attachment 177546 [details] [diff] [review]
Proposed patch

One comment:

>-    symKey = (PK11SymKey *)PORT_ZAlloc(sizeof(PK11SymKey));
>+    symKey = (PK11SymKey *)PORT_Alloc(sizeof(PK11SymKey));

I would have taken that opportunity to change it to the cleaner:

>+    symKey = PORT_New(PK11SymKey);

Comment 4

[Wan-Teh Chang]

Attachment: Use PORT_New and PORT_ZNew in pk11skey.c

Nelson, I knew you'd suggest that, and I actually
considered that.  But I saw the uses of PORT_Alloc
to allocate structures that way all over the place,
so I didn't make that change.

Since you brought it up, here is a code cleanup
patch for pk11skey.c.

Comment 5

[Nelson Bolyard (seldom reads bugmail)]

Comment on attachment 177629 [details] [diff] [review]
Use PORT_New and PORT_ZNew in pk11skey.c

r=nelson
I guess it's a good thing if I'm consitent enough about this that people can
predict my review comments. :)

Comment 6

[Wan-Teh Chang]

Comment on attachment 177629 [details] [diff] [review]
Use PORT_New and PORT_ZNew in pk11skey.c

I checked in this code cleanup patch on the trunk for NSS
3.10.

Checking in pk11skey.c;
/cvsroot/mozilla/security/nss/lib/pk11wrap/pk11skey.c,v  <--  pk11skey.c
new revision: 1.93; previous revision: 1.92
done

Comment 7

[Robert Relyea]

Comment on attachment 177546 [details] [diff] [review]
Proposed patch

r+ relyea (patchalready checked in long ago)