On Mon, 25 Sep 2023 15:29:42 GMT, Weijun Wang <wei...@openjdk.org> wrote:
>> Mark Powers has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> comment from Weijun
>
> test/jdk/sun/security/x509/X509CRLImpl/UnexpectedNPE.java line 27:
>
>> 25: * @test
>> 26: * @library /test/lib
>> 27: * @bug 5052433 8315042
>
> Usually we put `@bug` before `@library`.
Fixed.
> test/jdk/sun/security/x509/X509CRLImpl/UnexpectedNPE.java line 41:
>
>> 39: private static final String in =
>> "MAsGCSqGSMP7TQEHAjI1Bgn///////8wCwUyAQ==";
>> 40:
>> 41: public UnexpectedNPE() {}
>
> Useless constructor.
Removed.
> test/jdk/sun/security/x509/X509CRLImpl/UnexpectedNPE.java line 47:
>
>> 45: byte[] encoded_2 = { 0x30, 0x01, 0x00, 0x00 };
>> 46: byte[] encoded_3 = { 0x30, 0x01, 0x00 };
>> 47: byte[] decodedBytes = Base64.getDecoder().decode(in);
>
> Maybe rename to `encoded_4`?
Done.
> test/jdk/sun/security/x509/X509CRLImpl/UnexpectedNPE.java line 49:
>
>> 47: byte[] decodedBytes = Base64.getDecoder().decode(in);
>> 48:
>> 49: UnexpectedNPE unpe = new UnexpectedNPE() ;
>
> Just modify `run` to static. No class instance needed.
Good suggestion.
> test/jdk/sun/security/x509/X509CRLImpl/UnexpectedNPE.java line 60:
>
>> 58: if (cf == null) {
>> 59: try {
>> 60: cf = CertificateFactory.getInstance("X.509", "SUN");
>
> Make `cf` static and move the line above to `main`. Or, make it a local
> variable in `main` and pass it to all `run` calls.
Doing the former.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/15844#discussion_r1343103788
PR Review Comment: https://git.openjdk.org/jdk/pull/15844#discussion_r1343102818
PR Review Comment: https://git.openjdk.org/jdk/pull/15844#discussion_r1343102946
PR Review Comment: https://git.openjdk.org/jdk/pull/15844#discussion_r1343102629
PR Review Comment: https://git.openjdk.org/jdk/pull/15844#discussion_r1343103415
