On Tue, 5 Nov 2024 04:54:15 GMT, Prasadrao Koppula <pkopp...@openjdk.org> wrote:
>> Using SharedSecrets, I attempted to expose FileInputStream::path
>> information. After implementing the fix, I validated the startup performance
>> tests. Observed no consistent pattern of performance drops or gains, can
>> disregard the occasional performance drop observed in 1 or 2 runs.
>
> Prasadrao Koppula has updated the pull request incrementally with one
> additional commit since the last revision:
>
> Removed unnecessary code
src/java.base/share/classes/com/sun/crypto/provider/JceKeyStore.java line 700:
> 698: byte[] encoded = null;
> 699: int trustedKeyCount = 0, privateKeyCount = 0, secretKeyCount
> = 0;
> 700: String storeName = null;
let's initialize this to an empty string ("") -- otherwise you run the risk of
printing "null" in the debug output later.
src/java.base/share/classes/com/sun/crypto/provider/JceKeyStore.java line 709:
> 707: .getPath(stream);
> 708: if (keystorePath != null) {
> 709: storeName = Paths.get(keystorePath).getFileName()
`Path.of ` might be better
src/java.base/share/classes/javax/net/ssl/TrustManagerFactory.java line 286:
> 284: .getPath(ks);
> 285: if (keystorePath != null) {
> 286: SSLLogger.fine(provider.getName() + ": using \"" +
> Paths.get(
`Path.of ` might be better
src/java.base/share/classes/sun/security/pkcs12/PKCS12KeyStore.java line 1962:
> 1960: macAlgorithm = null;
> 1961: macIterationCount = -1;
> 1962: String storeName = null;
same here - you'll need to use "" to avoid printing null for non-FIS based
keystores
src/java.base/share/classes/sun/security/provider/JavaKeyStore.java line 663:
> 661: byte[] encoded;
> 662: int trustedKeyCount = 0, privateKeyCount = 0;
> 663: String storeName = null;
here also
src/java.base/share/classes/sun/security/util/KeyStoreDelegator.java line 293:
> 291: .getPath(stream);
> 292: String storeName = (keystorePath != null)
> 293: ?
> Paths.get(keystorePath).getFileName().toString()
suggestion `Path.of`
src/java.base/share/classes/sun/security/util/KeyStoreDelegator.java line 294:
> 292: String storeName = (keystorePath != null)
> 293: ?
> Paths.get(keystorePath).getFileName().toString()
> 294: : null;
avoid use of null
test/jdk/java/security/KeyStore/LogKeyStorePathVerifier.java line 67:
> 65: BufferedInputStream bis = new BufferedInputStream(
> 66: new FileInputStream(bisKeyStoreName));
> 67: BufferedInputStream bbis = new BufferedInputStream(bis)) {
you should create bbis from a unique file name - otherwise, you've no way of
distinguishing the output in the OutputAnalyzer tests later.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/20414#discussion_r1829577618
PR Review Comment: https://git.openjdk.org/jdk/pull/20414#discussion_r1829584187
PR Review Comment: https://git.openjdk.org/jdk/pull/20414#discussion_r1829584597
PR Review Comment: https://git.openjdk.org/jdk/pull/20414#discussion_r1829612196
PR Review Comment: https://git.openjdk.org/jdk/pull/20414#discussion_r1829612937
PR Review Comment: https://git.openjdk.org/jdk/pull/20414#discussion_r1829616007
PR Review Comment: https://git.openjdk.org/jdk/pull/20414#discussion_r1829618551
PR Review Comment: https://git.openjdk.org/jdk/pull/20414#discussion_r1829625052
