RFR: JDK-8291509 Minor cleanup could be done in sun.security [v7]

[Mark Powers]

On Wed, 7 Sep 2022 01:41:14 GMT, Weijun Wang <weijun at openjdk.org> wrote:

>> Mark Powers has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   comments from Max and Sean
>
> src/java.base/share/classes/sun/security/ssl/ECDHClientKeyExchange.java line 289:
> 
>> 287:             }
>> 288: 
>> 289:             // Can't figure this out, bail.
> 
> Maybe we should instead check if `namedParams` is null on line 286 and avoid the NPE there.

Fixed. Throwing exception sooner rather than later.

> src/java.base/share/classes/sun/security/ssl/SSLCipher.java line 70:
> 
>> 68:     @SuppressWarnings({"unchecked", "rawtypes"})
>> 69:     B_NULL("NULL", NULL_CIPHER, 0, 0, 0, 0, true, true,
>> 70:             new Map.Entry[] {
> 
> It looks cleaner to me if `new Map.Entry[] {` is put at the end of the previous line. Otherwise, the indentation seems growing backwards. Same for line 80 and all below.

I did as you suggested and did a bit more with the indentation.

> src/java.base/share/classes/sun/security/ssl/SSLExtensions.java line 307:
> 
>> 305:                                                     // extension_data length(2)
>> 306:                 }
>> 307:                 encodedLength += encoded.length + 4;
> 
> I think the two comment lines above should follow.

Sure that looks right?

> src/java.base/share/classes/sun/security/ssl/SSLHandshakeBinding.java line 2:
> 
>> 1: /*
>> 2:  * Copyright (c) 2018, 2022, Oracle and/or its affiliates. All rights reserved.
> 
> No change in this file?

Fixed. No change to file.

> src/java.base/share/classes/sun/security/ssl/SSLLogger.java line 587:
> 
>> 585:                 for (String string : strings) {
>> 586:                     builder.append("      \"" + string + "\"");
>> 587:                     if (!Objects.equals(string, strings[strings.length - 1])) {
> 
> The original usage looks weird. I'd rather use
> 
>                 int len = strings.length;
>                 for (int i = 0; i < len; i++) {
>                     String string = strings[i];
>                     builder.append("      "" + string + """);
>                     if (i != len - 1) {
>                         builder.append(",");
>                     }
>                     builder.append("\n");
>                 }

Fixed.

> src/java.base/share/classes/sun/security/ssl/SSLSocketFactoryImpl.java line 86:
> 
>> 84:     @Override
>> 85:     public Socket createSocket(String host, int port)
>> 86:     throws IOException {
> 
> Join this with the previous line.

Fixed.

> src/java.base/share/classes/sun/security/ssl/TransportContext.java line 587:
> 
>> 585:         boolean useUserCanceled = !isNegotiated &&
>> 586:                 (handshakeContext != null) && !peerUserCanceled;
>> 587:         // initial handshake
> 
> Move this comment line above a little.

Fixed.

-------------

PR: https://git.openjdk.org/jdk/pull/9972