On Mon, 2 Oct 2023 20:58:39 GMT, Weijun Wang <[email protected]> wrote:
>> Mark Powers has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> more comments from Weijun
>
> test/jdk/sun/security/x509/X509CRLImpl/UnexpectedNPE.java line 27:
>
>> 25: * @test
>> 26: * @bug 5052433 8315042
>> 27: * @summary NullPointerException for generateCRL and generateCRLs
>> methods.
>
> I didn't notice the `@summary` mention both methods. Now I'm not sure which
> one is better. You can either update this summary to only mention one or
> revert the code to check both.
I'll revert the code to check both.
> test/jdk/sun/security/x509/X509CRLImpl/UnexpectedNPE.java line 47:
>
>> 45: "MAsGCSqGSMP7TQEHAjI1Bgn///////8wCwUyAQ==");
>> 46:
>> 47: if (cf == null) {
>
> No need to check `if (cf == null)` or catch any exception. Just leave it
> uncaught.
Fixed.
> test/jdk/sun/security/x509/X509CRLImpl/UnexpectedNPE.java line 67:
>
>> 65: () -> cf.generateCRLs(new ByteArrayInputStream(buf)),
>> 66: CRLException.class);
>> 67: System.out.println("NPE checking passed");
>
> No need to print any message.
Less is better in my opinion.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/15844#discussion_r1343211943
PR Review Comment: https://git.openjdk.org/jdk/pull/15844#discussion_r1343211667
PR Review Comment: https://git.openjdk.org/jdk/pull/15844#discussion_r1343212046
