On Thu, 2 Oct 2025 18:49:53 GMT, Mark Powers <[email protected]> wrote:
>> [JDK-8343232](https://bugs.openjdk.org/browse/JDK-8343232) > > Mark Powers has updated the pull request incrementally with one additional > commit since the last revision: > > more review comments from Weijun and Sean src/java.base/share/classes/com/sun/crypto/provider/PBMAC1Parameters.java line 75: > 73: > 74: // the key derivation function (default is HmacSHA1) > 75: private final ObjectIdentifier kdfAlgo_OID = This can just be a local variable in `getEncoded`. src/java.base/share/classes/com/sun/crypto/provider/PBMAC1Parameters.java line 81: > 79: private int keyLength = -1; > 80: > 81: protected void Init(AlgorithmParameterSpec paramSpec) You don't need this method/ctor anymore, only the one that takes a `byte[]`. src/java.base/share/classes/com/sun/crypto/provider/PBMAC1Parameters.java line 97: > 95: + "not an ASN.1 SEQUENCE tag"); > 96: } > 97: DerValue[] Info = (new > DerInputStream(pBMAC1_params.toByteArray())) No need for parens around `new DerInputStream`. Also, s/Info/info/ src/java.base/share/classes/com/sun/crypto/provider/PBMAC1Parameters.java line 138: > 136: } > 137: > 138: protected byte[] getEncoded() throws IOException { The methods don't need to be `protected`. Make them package-private after you move the class to `sun.security.pkcs12`. src/java.base/share/classes/com/sun/crypto/provider/PBMAC1Parameters.java line 172: > 170: protected byte[] getEncoded(String encodingMethod) throws > IOException { > 171: return getEncoded(); > 172: } This method doesn't seem useful or needed. src/java.base/share/classes/sun/security/pkcs12/MacData.java line 179: > 177: } > 178: > 179: static Mac getMac(String macAlgorithm, char[] password, This method can be private. src/java.base/share/classes/sun/security/pkcs12/MacData.java line 204: > 202: try { > 203: if (macAlgorithm.startsWith("PBEWith")) { > 204: m.init(pbeKey); should be 4 spaces indent instead of 5 spaces, also on line 206. src/java.base/share/classes/sun/security/pkcs12/MacData.java line 244: > 242: * create a message authentication code (MAC) > 243: */ > 244: public static byte[] calculateMac(char[] passwd, byte[] data, Method should be package-private. src/java.base/share/classes/sun/security/util/PBKDF2Parameters.java line 84: > 82: > 83: // the pseudorandom function (default is HmacSHA1) > 84: private ObjectIdentifier kdfAlgo_OID = I agree with Valerie that it looks like it can be a local variable inside the constructor on line 125. src/java.base/share/classes/sun/security/util/PBKDF2Parameters.java line 89: > 87: private String prfAlgo = "HmacSHA1"; > 88: > 89: public PBKDF2Parameters(DerValue keyDerivationFunc) throws > IOException { Add some comments describing this ctor and its parameter. src/java.base/share/classes/sun/security/util/PBKDF2Parameters.java line 148: > 146: * > 147: * @return the salt. Returns a new array > 148: * each time this method is called. Does not return a new array each time called so remove this comment. ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/24429#discussion_r2408034141 PR Review Comment: https://git.openjdk.org/jdk/pull/24429#discussion_r2408043209 PR Review Comment: https://git.openjdk.org/jdk/pull/24429#discussion_r2408046719 PR Review Comment: https://git.openjdk.org/jdk/pull/24429#discussion_r2408070382 PR Review Comment: https://git.openjdk.org/jdk/pull/24429#discussion_r2408067504 PR Review Comment: https://git.openjdk.org/jdk/pull/24429#discussion_r2408174703 PR Review Comment: https://git.openjdk.org/jdk/pull/24429#discussion_r2408165609 PR Review Comment: https://git.openjdk.org/jdk/pull/24429#discussion_r2408206621 PR Review Comment: https://git.openjdk.org/jdk/pull/24429#discussion_r2407354430 PR Review Comment: https://git.openjdk.org/jdk/pull/24429#discussion_r2407440344 PR Review Comment: https://git.openjdk.org/jdk/pull/24429#discussion_r2407364493
