On Thu, 21 Oct 2021 02:22:18 GMT, Weijun Wang <wei...@openjdk.org> wrote:
>> Sean Mullan has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> - Skip digest alg decomposing check for algorithms that don't contain
>> "SHA".
>> - Remove hasLoop method and fold code into decomposeName method.
>
> src/java.base/share/classes/sun/security/provider/certpath/AlgorithmChecker.java
> line 80:
>
>> 78: private final Date date;
>> 79: private PublicKey prevPubKey;
>> 80: private final String variant;
>
> You can group fields to final and non-final ones.
Ok.
> src/java.base/share/classes/sun/security/provider/certpath/AlgorithmChecker.java
> line 174:
>
>> 172: }
>> 173:
>> 174: @Override
>
> In the `init()` method below, just write `prevPubKey = trustedPubKey` no
> matter if `trustedPubKey` is null or not.
Yes.
> src/java.base/share/classes/sun/security/provider/certpath/AlgorithmChecker.java
> line 318:
>
>> 316: prevPubKey = currPubKey;
>> 317: return;
>> 318: }
>
> I find it more natural to enclose the block below in a `if (prevPubKey !=
> null)` block, and you only need to call one `prevPubKey = currPubKey` at the
> end.
Ok.
> src/java.base/share/classes/sun/security/provider/certpath/AlgorithmChecker.java
> line 362:
>
>> 360: // Don't bother if the check has started or trust anchor has
>> already
>> 361: // been specified.
>> 362: if (this.prevPubKey == null) {
>
> Maybe it's cleaner to write `if (this.trustedPubKey == null)` above. Anyway,
> after `init()` they are the same.
Agree.
> src/java.base/share/classes/sun/security/provider/certpath/AlgorithmChecker.java
> line 363:
>
>> 361: // been specified.
>> 362: if (this.prevPubKey == null) {
>> 363: if (anchor == null) {
>
> This won't happen. Or, you can ignore it.
>
> This makes it possible to call this method in the constructor.
Ok, will remove. But I will keep this method separate since, unlike the ctor it
needs to check if `trustedPubKey` is `null` before setting the `prevPubKey`.
-------------
PR: https://git.openjdk.java.net/jdk/pull/5928
