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

[Weijun Wang]

On Thu, 1 Sep 2022 02:26:48 GMT, Mark Powers <mpowers at 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