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

[Mark Powers]

On Tue, 6 Sep 2022 16:24:01 GMT, Sean Mullan <mullan at openjdk.org> wrote:

>> Mark Powers has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   white space
>
> src/java.base/share/classes/sun/security/provider/AbstractDrbg.java line 81:
> 
>> 79:      * does <em>not</em> need to compare it to {@link #reseedInterval}.
>> 80:      *
>> 81:      * Volatile, will be used in a double-checked locking.
> 
> Also, remove "a" as it reads more correctly w/o it.

Fixed.

> src/java.base/share/classes/sun/security/provider/ConfigFile.java line 167:
> 
>> 165:             // call in a doPrivileged
>> 166:             //
>> 167:             // we have already passed the Configuration.getInstance
> 
> s/we/We/

Fixed.

> src/java.base/share/classes/sun/security/provider/ConfigFile.java line 168:
> 
>> 166:             //
>> 167:             // we have already passed the Configuration.getInstance
>> 168:             // security check.  also, this class is not freely accessible
> 
> s/also/Also/

Fixed.

> src/java.base/share/classes/sun/security/provider/PolicyParser.java line 1165:
> 
>> 1163:             }
>> 1164: 
>> 1165:             // everything matched -- the 2 objects are equal
> 
> remove the comment as it no longer applies

Fixed.

> src/java.base/share/classes/sun/security/provider/SunEntries.java line 369:
> 
>> 367:      * Use a URI to access this File. Previous code used a URL
>> 368:      * which is less strict on syntax. If we encounter a
>> 369:      * URISyntaxException we make the best efforts for backwards
> 
> I'd change this to "a best effort". Same comment on l392.

Fixed.

> src/java.base/share/classes/sun/security/provider/certpath/BuildStep.java line 245:
> 
>> 243:             out = out + vertex.moreToString();
>> 244:             break;
>> 245:             default:
> 
> align this with the "case" statements.

Fixed.

> src/java.base/share/classes/sun/security/provider/certpath/Builder.java line 62:
> 
>> 60:     /**
>> 61:      * Flag indicating whether support for the caIssuers field of the
>> 62:      * Authority Information Access extension shall be enabled. Currently,
> 
> I think it reads better w/o the comma.

Fixed.

> src/java.base/share/classes/sun/security/provider/certpath/CertId.java line 226:
> 
>> 224:                 "\nissuerKeyHash: \n" +
>> 225:                 encoder.encode(issuerKeyHash) +
>> 226:                 "\n" + certSerialNumber.toString();
> 
> I believe this creates more `String` objects whereas the previous code used a mutable `StringBuilder` to build up the `String` first. Not sure this code is better, even though this is probably not a commonly called method.

If the new code requires more work (CPU and memory) then maybe it is not a good idea. I'll try to find out more about this method before I revert the change.

> src/java.base/share/classes/sun/security/provider/certpath/DistributionPointFetcher.java line 337:
> 
>> 335:         if (pointCrlIssuers != null) {
>> 336:             if (idpExt == null ||
>> 337:                     idpExt.get (IssuingDistributionPointExtension.INDIRECT_CRL)
> 
> Nit, remove space after `get`.

Fixed.

> src/java.base/share/classes/sun/security/provider/certpath/DistributionPointFetcher.java line 338:
> 
>> 336:             if (idpExt == null ||
>> 337:                     idpExt.get (IssuingDistributionPointExtension.INDIRECT_CRL)
>> 338:                     == (Boolean.FALSE)) {
> 
> Don't need the parens around `Boolean.FALSE` now.

Fixed.

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

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