On Fri, 11 Sep 2026 18:49:46 GMT, Mark Powers <[email protected]> wrote:
>> https://bugs.openjdk.org/browse/JDK-8326087 >> >> --------- >> - [x] I confirm that I make this contribution in accordance with the >> [OpenJDK Interim AI Policy](https://openjdk.org/legal/ai). > > Mark Powers has updated the pull request incrementally with two additional > commits since the last revision: > > - comment from Weijun to eliminate huge indentation from if statement > - comment from Weijun about probe patterns src/java.base/share/classes/sun/security/pkcs12/PKCS12KeyStore.java line 2586: > 2584: (PKCS12_HEADER_PATTERNS[i][2] == > 2585: (finalPeek & PKCS12_HEADER_MASKS[i][2]))) { > 2586: result = true; Nit: Wouldn't it be a bit easier to read if this just returns true and the line 2591 returns false? test/jdk/sun/security/pkcs12/EmptyAuthSafeTest.java line 52: > 50: private static final String ks2 = "MBACAQMwCwYJKoZIhvcNAQcB"; > 51: > 52: public static void main(String[] args) throws Exception { This test structure will skip the rest of the test if there is a failure. I think it should be either a junit test or the errors should be stored and printed in the end. Personally, I think junit might be the easiest way. What do you think? test/jdk/sun/security/pkcs12/EmptyAuthSafeTest.java line 89: > 87: boolean actual = > PKCS12KeyStore.isPasswordless(keyStoreFile.toFile()); > 88: if (actual != expected) { > 89: throw new Exception("Expected isPasswordless() to return " Wouldn't `RuntimeException` fit better here and in the other parts of the test file? This is what is normally used ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/32357#discussion_r3996177908 PR Review Comment: https://git.openjdk.org/jdk/pull/32357#discussion_r3996170340 PR Review Comment: https://git.openjdk.org/jdk/pull/32357#discussion_r3996166226
