On Thu, 1 Jun 2023 21:17:11 GMT, Ben Perez <d...@openjdk.org> wrote:
> Fixed `engineInitSign` in `DSA.java` and added `SecureRandomReset.java` to
> DSA tests
src/java.base/share/classes/sun/security/provider/DSA.java line 1:
> 1: /*
Update the copyright year for this file.
src/java.base/share/classes/sun/security/provider/DSA.java line 163:
> 161: // reset signing random
> 162: this.signingRandom = null;
> 163: this.signingRandom = getSigningRandom();
First, no need to set twice.
Second, if you assign `getSigningRandom()` here, all other lazy
`getSigningRandom()` calls become useless.
test/jdk/sun/security/provider/DSA/SecureRandomReset.java line 28:
> 26: * @bug 8308474
> 27: * @summary Test that calling initSign resets RNG
> 28: * @run main SecureRandomReset
Not necessary if this is the only action in the test. Some people think this is
clearer, though. For me, I have copied an existing test to create a new one and
forgot to modify this line before.
test/jdk/sun/security/provider/DSA/SecureRandomReset.java line 45:
> 43: Signature s = Signature.getInstance("SHA256withDSA");
> 44:
> 45: //Initialize deterministic RNG and sign
We usually add a space after `//`.
test/jdk/sun/security/provider/DSA/SecureRandomReset.java line 53:
> 51: String sig2 = HexFormat.of().formatHex(s.sign());
> 52:
> 53: if (!sig1.equals(sig2)) {
If you only want to compare, there is no need to covert the byte arrays to
strings. Just call `Arrays.equals(l,r)`.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/14273#discussion_r1219733876
PR Review Comment: https://git.openjdk.org/jdk/pull/14273#discussion_r1219725409
PR Review Comment: https://git.openjdk.org/jdk/pull/14273#discussion_r1219732401
PR Review Comment: https://git.openjdk.org/jdk/pull/14273#discussion_r1219727531
PR Review Comment: https://git.openjdk.org/jdk/pull/14273#discussion_r1219726822
