On Thu, 1 Sep 2022 14:09:42 GMT, Weijun Wang <wei...@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/util/DerInputStream.java line 292:
>
>> 290: * a later call to <code>reset</code> will return here.
>> 291: * The {@code readAheadLimit} is useless here because
>> 292: * all data is available, and we can go to anywhere at will.
>
> I'd rather add a comma before `because`. The sentence means `because (A &&
> B)` and I don't want it look like `because (A) && B`.
Added comma.
> src/java.base/share/classes/sun/security/util/HexDumpEncoder.java line 157:
>
>> 155: int j;
>> 156: int numBytes;
>> 157: byte[] tmpbuffer = new byte[bytesPerLine()];
>
> Add one space in between.
Fixed.
> src/java.base/share/classes/sun/security/util/HexDumpEncoder.java line 268:
>
>> 266: int j;
>> 267: int numBytes;
>> 268: byte[] tmpbuffer = new byte[bytesPerLine()];
>
> Add one space in between. Or, is my GitHub view distorted?
Fixed.
> src/java.base/share/classes/sun/security/validator/PKIXValidator.java line
> 177:
>
>> 175: private void setDefaultParameters(String variant) {
>> 176: if ((Objects.equals(variant, Validator.VAR_TLS_SERVER)) ||
>> 177: (Objects.equals(variant, Validator.VAR_TLS_CLIENT))) {
>
> I think the original code is OK. We only use string literals for variants.
> That said, maybe it's better to use an enum.
I'll revert to the original code. An enum change would affect method
definitions wherever `Validator.VAR_:L_SERVER` and friends are used.
> src/java.base/share/classes/sun/security/validator/Validator.java line 268:
>
>> 266: // redundant.
>> 267: boolean checkUnresolvedCritExts =
>> 268: !Objects.equals(type, TYPE_PKIX);
>
> Same as above. We only use literals.
I'll revert to the original code.
-------------
PR: https://git.openjdk.org/jdk/pull/9972
