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
