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

Mark Powers mpowers at openjdk.org
Fri Sep 2 00:41:22 UTC 2022


On Thu, 1 Sep 2022 03:02:57 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 Sean and Max
>
> src/java.base/share/classes/sun/security/provider/certpath/DistributionPointFetcher.java line 339:
> 
>> 337:                 idpExt.get
>> 338:                     (IssuingDistributionPointExtension.INDIRECT_CRL).equals
>> 339:                         (Boolean.FALSE)) {
> 
> There are only 2 `Boolean` values, so it's safe to use `==`. Also, I'd like
> 
> idpExt.get(IssuingDistributionPointExtension.INDIRECT_CRL)
>         == Boolean.FALSE

Fixed.

> src/java.base/share/classes/sun/security/provider/certpath/DistributionPointFetcher.java line 572:
> 
>> 570:             if (interimReasonsMask[i] &&
>> 571:                     !(i < reasonsMask.length && reasonsMask[i]))
>> 572:             {
> 
> More brace to previous line.

Fixed.

> src/java.base/share/classes/sun/security/provider/certpath/ForwardBuilder.java line 544:
> 
>> 542:                     if (distanceTto1 == distanceTto2) {
>> 543:                         return -1;
>> 544:                     } else if (distanceTto1 > 0 && distanceTto2 <= 0) {
> 
> I feel there is no need for `distanceTto1 > 0 &&` in the above line.

You need it if `distanceTto1 `is 0 on line 541.

> src/java.base/share/classes/sun/security/provider/certpath/OCSPResponse.java line 703:
> 
>> 701:      *
>> 702:      * @return the {@code ResponderId} from this response or {@code null}
>> 703:      *      if no responder ID is in the body of the response e.g. a
> 
> I'd rather add a closed parenthesis at the end, or at least add a comma before `e.g.`

Adding a comma.

> src/java.base/share/classes/sun/security/provider/certpath/PolicyNodeImpl.java line 95:
> 
>> 93:         mChildren = new HashSet<>();
>> 94: 
>> 95:         mValidPolicy = Objects.requireNonNullElse(validPolicy, "");
> 
> I prefer `?:`.

Fixed.

> src/java.base/share/classes/sun/security/provider/certpath/RevocationChecker.java line 479:
> 
>> 477: 
>> 478:         switch (type) {
>> 479:         case "LDAP":
> 
> `case` lines should be indented.

Fixed.

> src/java.base/share/classes/sun/security/tools/PathList.java line 69:
> 
>> 67:         while (st.hasMoreTokens()) {
>> 68:             URL url = fileToURL(new File(st.nextToken()));
>> 69:             urls[count++] = url;
> 
> Please update the javadoc of `fileToURL` to remove `or null if unknown` in `@return`.

Fixed.

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

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



More information about the security-dev mailing list