On Fri, 8 Nov 2024 19:10:14 GMT, Mark Powers <mpow...@openjdk.org> wrote:
>> Weijun Wang has updated the pull request with a new target base due to a >> merge or a rebase. The incremental webrev excludes the unrelated changes >> brought in by the merge/rebase. The pull request contains 13 additional >> commits since the last revision: >> >> - Merge branch 'master' into 8342442 >> - more precise ML-DSA sigVer negative test >> - more clear exception message >> - check provider availability; SourceRandom in lib; use Utils.toByteArray >> - rename property names, add an example >> - acvp.test.alg system property >> - remove plugin design >> - rename >> - grammar issues >> - license >> - ... and 3 more: https://git.openjdk.org/jdk/compare/ea3e4a63...a7e22873 > > test/jdk/sun/security/provider/acvp/Launcher.java line 77: > >> 75: var p = Security.getProvider(provProp); >> 76: if (p == null) { >> 77: System.err.println(provProp + " is not a registered >> provider name"); > > Seems odd to print the error message and throw an exception with the same > message. This is in a static block and the exception message does not show up when I run it with `jtreg`, so I print the message. Even so, I think the exception message had better to be the same. > test/jdk/sun/security/provider/acvp/ML_DSA_Test.java line 85: > >> 83: public byte[] getEncoded() { return >> toByteArray(c.get("sk").asString()); } >> 84: }; >> 85: var sr = new FixedSecureRandom(det ? new byte[32] : >> toByteArray(c.get("rnd").asString())); > > This line and others are greater than 80 characters. I'll shorten some. I'd still like to keep the `new PrivateKey()` block compact. ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/21548#discussion_r1835093034 PR Review Comment: https://git.openjdk.org/jdk/pull/21548#discussion_r1835094158
