On Thu, 1 Sep 2022 02:26:48 GMT, Mark Powers <mpow...@openjdk.org> wrote:
>> https://bugs.openjdk.org/browse/JDK-8291509 > > Mark Powers has updated the pull request incrementally with one additional > commit since the last revision: > > comments from Sean and Max More comments. `x509` and `ssl` not reviewed yet. src/java.base/share/classes/sun/security/util/AlgorithmDecomposer.java line 110: > 108: !elements.contains(e.getKey())) { > 109: elements.add(e.getKey()); > 110: } else if (elements.contains(e.getKey())) { I assume this means that the `add()` method will behave the same anyway, but the original code does seem more clear to me. 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`. 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. 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? 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. 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. ------------- PR: https://git.openjdk.org/jdk/pull/9972
