Code Review Request for 7146728 and 7130959

[Valerie (Yu-Ching) Peng]

Ok, more comments/changes added as suggested.
Webrev updated at:
http://cr.openjdk.java.net/~valeriep/7146728/webrev.02/

Thanks,
Valerie

On 03/08/12 17:15, Brad Wetmore wrote:
>
> >> 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