code review request: 8012971 PKCS11Test hiding exception failures

Anthony Scarpino anthony.scarpino at oracle.com
Thu Jul 18 22:17:04 UTC 2013


On 07/18/2013 02:49 PM, Valerie (Yu-Ching) Peng wrote:
>
> Please find comments inline.
>
> On 07/17/13 13:51, Anthony Scarpino wrote:
>> I have broken these into two webrev.  The first:
>>
>> JDK-8012971 PKCS11Test hiding exception failures
>> http://cr.openjdk.java.net/~ascarpino/8012971/webrev.01/
>>
>> handles the minimum changes needed for PKCS11Test to function and the
>> Problemlist updated to reflect the failures that would show up.
> Looks fine in general, I only have some nit comments, and questions
> since I have not touched NSS much.
> PKCS11Test.java
> 1) line 88, change "this finally block" to "any finally block"
yep

> 2) line 192, add space before and after the "+" for consistency with the
> rest of the source, e.g. line 213.
yep

> 3) The comments from line 58-60 say the default value is "nss3" but the
> default value set on line 61 is "softokn3". Seems conflicting?

yep.  it nss3 use to be the default, must have forgot the change the comment

> 4) Is "nss3" only used for Secmod? It seems that everywhere else you use
> the "softokn3" value. If yes, perhaps using "setSecmod()" would be
> clearer than "useNSS()".

Secmod is the only one currently, but I would rather leave it as NSS 
because I named it for the library it's looking for.  If another test 
comes around that is not a Secmod test, then setSecmod() doesn't seem 
logical.

webrev is updated in place.

>
> Will look at 8020424 later today.
> Thanks,
> Valerie
>>
>>
>> The second, are the test changes that fix the problems uncovered by
>> the above change:
>>
>> JDK-8020424 The NSS version should be detected before running crypto
>> tests
>> http://cr.openjdk.java.net/~ascarpino/8020424/webrev.00/
>>
>> Tony
>>
>




More information about the security-dev mailing list