On Thu, 8 May 2025 11:38:55 GMT, Weijun Wang <wei...@openjdk.org> wrote:
>> Mikhail Yankelevich has updated the pull request incrementally with one
>> additional commit since the last revision:
>>
>> Rajan's comments
>
> test/jdk/java/security/cert/CertificateFactory/SlowStream.java line 28:
>
>> 26: * @bug 6813340
>> 27: * @summary X509Factory should not depend on is.available()==0
>> 28: * @run main/othervm SlowStream
>
> Why `othervm`?
Removed in the next commit
> test/jdk/java/security/cert/CertificateFactory/SlowStream.java line 53:
>
>> 51: "pem"))) {
>> 52:
>> 53: final byte[] buffer = new byte[4096];
>
> This test was probably written very log ago where there was no NIO or
> `InputStream::readAllBytes`. The file itself is quite small and there is no
> need to read in blocks. You can use modern methods like `Files::readAllBytes`
> or `InputStream::transferTo`.
changed in the next commit
> test/jdk/java/security/cert/CertificateFactory/SlowStream.java line 75:
>
>> 73: try {
>> 74: final var factory =
>> CertificateFactory.getInstance("X.509");
>> 75: final var factorySize =
>> factory.generateCertificates(inputStream).size();
>
> Please change the variable name to something like `numOfCerts`.
Changed in the next commit
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/23394#discussion_r2081399746
PR Review Comment: https://git.openjdk.org/jdk/pull/23394#discussion_r2081399549
PR Review Comment: https://git.openjdk.org/jdk/pull/23394#discussion_r2081399274
