Code Review Request for 7146728 and 7130959
Brad Wetmore
bradford.wetmore at oracle.com
Fri Mar 9 01:15:32 UTC 2012
>> DHKeyAgreement:
>> ===============
> I added more comments to both code blocks to explain what's going on.
A couple more requested comments.
Around line 299, could please add that the sign bit is always 0.
Around line 303, could you put in a comment that since there is enough
room here for the sign bit in this array, it just becomes a leading zero.
> Good point, I modified the engineGenerateSecret() to leverage
> engineGenerateSecret(byte[], int) method.
Great, thanks!
>> P11KeyAgreement.java
>> ====================
>>
>> // Solaris
>>
>> Shouldn't this be "// Solaris/NSS", for the case in which there isn't
>> a leading zero?
> Well, I think putting NSS in both places seems somewhat confusing. What
> I tried to point out is that NSS (at least in the older versions, I have
> not tried the newer version to see if they switched behavior) trims off
> the leading 0s whenever possible. I've made some modification hoping to
> make it clearer.
Much better, thanks!
Minor nit (take or leave) you could adjust the logic around 208 to only
create newSecret if secret.length < secretLen.
if (secret.length > secretLen) {
throw new ProviderException(...
}
byte [] newSecret = new byte[secretLen];
System.arraycopy(...);
return newSecret;
>
>> TestShort.java
>> ==============
>> New bugid
>>
>> @bug 4942494 7146728
> Done.
>
>> Indention problem around the try.
> Fixed.
Thanks for taking out the commented out code, I almost mentioned it and
then didn't.
Brad
More information about the security-dev
mailing list