On Mon, 29 Aug 2022 21:48:18 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: > > white space Files before (and equals to) `X509Factory`. src/java.base/share/classes/sun/security/jca/ProviderList.java line 672: > 670: "SHA3-384", "SHA3-512" }; > 671: private static final String[] HmacSHA3Group = { "HmacSHA3-224", > 672: "HmacSHA3-256", "HmacSHA3-384", "HmacSHA3-512"}; Shall I change these `static final` fields to UPPERCASE? src/java.base/share/classes/sun/security/pkcs/PKCS7.java line 216: > 214: > 215: private void parseNetscapeCertChain(DerValue val) > 216: throws IOException { No need for a newline here. src/java.base/share/classes/sun/security/pkcs/PKCS7.java line 263: > 261: // signerInfos SignerInfos } > 262: private void parseSignedData(DerValue val) > 263: throws IOException { No need for a newline here. src/java.base/share/classes/sun/security/pkcs/PKCS7.java line 389: > 387: */ > 388: private void parseOldSignedData(DerValue val) > 389: throws IOException No need for a newline here. src/java.base/share/classes/sun/security/pkcs/SignerInfo.java line 106: > 104: */ > 105: public SignerInfo(DerInputStream derin) > 106: throws IOException { No need for a newline here. src/java.base/share/classes/sun/security/pkcs/SignerInfo.java line 121: > 119: */ > 120: public SignerInfo(DerInputStream derin, boolean oldStyle) > 121: throws IOException { Indent 4 more spaces. src/java.base/share/classes/sun/security/pkcs10/PKCS10.java line 369: > 367: private final PKCS10Attributes attributeSet; > 368: private byte[] encoded; // signed > 369: } Either realign all the names or none. src/java.base/share/classes/sun/security/pkcs10/PKCS10Attributes.java line 184: > 182: return false; > 183: PKCS10Attribute thisAttr, otherAttr; > 184: String key; This whole `toString` method can be enhanced. Can we just compare `this.map.equals(that.map)`? src/java.base/share/classes/sun/security/pkcs10/PKCS10Attributes.java line 216: > 214: */ > 215: public String toString() { > 216: return map.size() + "\n" + map.toString(); `toString` not needed. src/java.base/share/classes/sun/security/pkcs12/MacData.java line 47: > 45: > 46: private String digestAlgorithmName; > 47: private AlgorithmParameters digestAlgorithmParams; Yes, this field is not used now. But one day we might see one that has parameters. I'd rather keep them and maybe fix `getEncoded` by then. src/java.base/share/classes/sun/security/pkcs12/MacData.java line 58: > 56: */ > 57: MacData(DerInputStream derin) > 58: throws IOException { No need for a newline here. src/java.base/share/classes/sun/security/pkcs12/PKCS12KeyStore.java line 1723: > 1721: if (entry instanceof PrivateKeyEntry keyEntry) { > 1722: certs = Objects.requireNonNullElseGet(keyEntry.chain, > () -> > 1723: new Certificate[0]); I'd rather use `?:`. src/java.base/share/classes/sun/security/pkcs12/PKCS12KeyStore.java line 1809: > 1807: */ > 1808: private byte[] createSafeContent() > 1809: throws IOException { No need for a newline here. src/java.base/share/classes/sun/security/pkcs12/PKCS12KeyStore.java line 2213: > 2211: if (entry.keyId != null) { > 2212: ArrayList<X509Certificate> chain = > 2213: new ArrayList<>(); No need for a newline here. src/java.base/share/classes/sun/security/pkcs12/PKCS12KeyStore.java line 2389: > 2387: private void loadSafeContents(DerInputStream stream) > 2388: throws IOException, CertificateException > 2389: { Put brace to previous line. src/java.base/share/classes/sun/security/pkcs12/PKCS12KeyStore.java line 2554: > 2552: Long.parseLong(keyIdStr.substring(5))); > 2553: } catch (Exception e) { > 2554: date = null; Maybe add a comment? src/java.base/share/classes/sun/security/provider/DomainKeyStore.java line 564: > 562: } > 563: return new AbstractMap.SimpleEntry<>("", > 564: Collections.emptyList()); No need for a newline here. src/java.base/share/classes/sun/security/provider/PolicyParser.java line 576: > 574: } > 575: > 576: return (ignoreEntry == true) ? null : e; Remove definition of `ignoreEntry`. src/java.base/share/classes/sun/security/provider/PolicyParser.java line 956: > 954: ge.principals = new LinkedList<>(this.principals); > 955: ge.permissionEntries = > 956: new Vector<>(this.permissionEntries); No need for a newline here. src/java.base/share/classes/sun/security/provider/SecureRandom.java line 235: > 233: if (r > 0) { > 234: // How many bytes? > 235: todo = Math.min((result.length - index), (DIGEST_SIZE - r)); No need for so many parentheses. ------------- PR: https://git.openjdk.org/jdk/pull/9972
