517590 - Document how to free the attribute value returned by PK11_ReadRawAttribute

Status: RESOLVED FIXED

(NSS :: Documentation, defect, P2)

Description

[Wan-Teh Chang]

Attachment: Proposed patch

The attached patch adds a comment to pk11pub.h to document
how the caller of PK11_ReadRawAttribute should free the
attribute value returned by PK11_ReadRawAttribute.

Comment 1

[Wan-Teh Chang]

Attachment: Proposed patch v2

Also document how to free the return value of PK11_DEREncodePublicKey.

Comment 2

[Wan-Teh Chang]

Attachment: Document SEC_ASN1EncodeItem

Comment 3

[Robert Relyea]

Comment on attachment 401554 [details] [diff] [review]
Proposed patch v2

r+ the need to free PK11_DEREncodePublicKey should be clear from the lack of const (though it's true the we don't always include const where we need to).

The readRaw, however, is not at all clear without looking that the source code.

r+ one doesn't hurt, and the other is necessary.

bob

Comment 4

[Robert Relyea]

Comment on attachment 401565 [details] [diff] [review]
Document SEC_ASN1EncodeItem

r+, though, this is a very common semantic in NSS for certain allocation functions. It might be best to have a single place that describes the this semantic and give it a name, then point everything else to it...

(something like NSS flexible allocation).

bob

(also I knot you give both options to free the data portion (SECITEM_FreeItem with false and PORT_Free(dest->data)). I'm wondering if we should just document the former, and if we need to doc both, shouldn't we doc both in the PK11_ReadRaw case as well?

bob

Comment 5

[Wan-Teh Chang]

Yes, I also document both options to free the data portion in the
PK11_ReadRawAttribute case.

I know the comments I added are not the best way to document this.
It's just that my coworkers are starting to venture outside libSSL
(see http://codereview.chromium.org/208032), and it's embarassing
that many of the functions they're using are not documented.  I
plan to add a man page for each of these functions, and the comments
I added will be copied to the man pages.

Checking in lib/pk11wrap/pk11pub.h;
/cvsroot/mozilla/security/nss/lib/pk11wrap/pk11pub.h,v  <--  pk11pub.h
new revision: 1.31; previous revision: 1.30
done
Checking in lib/util/secasn1.h;
/cvsroot/mozilla/security/nss/lib/util/secasn1.h,v  <--  secasn1.h
new revision: 1.17; previous revision: 1.16
done