518457 - SECKEY_EncodeDERSubjectPublicKeyInfo and PK11_DEREncodePublicKey are duplicate

Status: RESOLVED FIXED

(NSS :: Libraries, defect)

Description

[Wan-Teh Chang]

Attachment: Proposed patch

SECKEY_EncodeDERSubjectPublicKeyInfo and PK11_DEREncodePublicKey
have the same function prototypes and implementations (except
that PK11_DEREncodePublicKey has a leak, see bug 518446).

We should implement one by calling the other to reduce duplicate
code.  Since lib/cryptohi appears to be at a higher layer than
lib/pk11wrap, I decided to have SECKEY_EncodeDERSubjectPublicKeyInfo
call PK11_DEREncodePublicKey.

Comment 1

[Nelson Bolyard (seldom reads bugmail)]

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

r=nelson.
The PK11 implementation is much cleaner, too.

Comment 2

[Wan-Teh Chang]

Nelson, thanks for the code review.  The two implementations
are copies of each other because the comments and coding styles
are exactly the same.

Comment 3

[Wan-Teh Chang]

I checked in the patch on the NSS trunk (NSS 3.12.5).

Checking in seckey.c;
/cvsroot/mozilla/security/nss/lib/cryptohi/seckey.c,v  <--  seckey.c
new revision: 1.50; previous revision: 1.49
done

Comment 4

[Wan-Teh Chang]

Attachment: Do the opposite

On closer inspection, I think it's better to let
SECKEY_EncodeDERSubjectPublicKeyInfo have the
real implementation because the symmetric function
SECKEY_DecodeDERSubjectPublicKeyInfo exists.  This
allows us to contrast the Encode and Decode code.

pk11akey.c already includes keyhi.h (via key.h),
which includes keyhi.h, so it's fine for
PK11_DEREncodePublicKey to call
SECKEY_EncodeDERSubjectPublicKeyInfo.

Checking in cryptohi/seckey.c;
/cvsroot/mozilla/security/nss/lib/cryptohi/seckey.c,v  <--  seckey.c
new revision: 1.51; previous revision: 1.50
done
Checking in pk11wrap/pk11akey.c;
/cvsroot/mozilla/security/nss/lib/pk11wrap/pk11akey.c,v  <--  pk11akey.c
new revision: 1.29; previous revision: 1.28
done