On Tue, 6 Sep 2022 16:24:01 GMT, Sean Mullan <mul...@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
