On Wed, 15 Apr 2026 04:58:57 GMT, Valerie Peng <[email protected]> wrote:
>> Initial commit containing the public API changes and related regression >> tests. >> >> --------- >> - [x] I confirm that I make this contribution in accordance with the >> [OpenJDK Interim AI Policy](https://openjdk.org/legal/ai). > > Valerie Peng has updated the pull request incrementally with one additional > commit since the last revision: > > additional data cleanup. Some initial feedback, will continue reviewing next week. src/java.base/share/classes/javax/crypto/spec/Argon2ParameterSpec.java line 49: > 47: * > 48: * @since 27 > 49: */ Add an `@see KDF` as a helpful link. src/java.base/share/classes/javax/crypto/spec/Argon2ParameterSpec.java line 54: > 52: > 53: /** > 54: * Version of Argon2 implementations Add period at end of sentence. src/java.base/share/classes/javax/crypto/spec/Argon2ParameterSpec.java line 166: > 164: > 165: /** > 166: * Set the memory value to the builder. This doesn't read right. I'd suggest rewording as "Sets the memory parameter of this builder." Same comment for similar methods. src/java.base/share/classes/javax/crypto/spec/Argon2ParameterSpec.java line 171: > 169: * @return this builder > 170: * @throws IllegalArgumentException > 171: * if {@code m} is less than 8 or less than 8 * {@code > p} if s/if/; or if/ (2nd instance of if) src/java.base/share/classes/javax/crypto/spec/Argon2ParameterSpec.java line 172: > 170: * @throws IllegalArgumentException > 171: * if {@code m} is less than 8 or less than 8 * {@code > p} if > 172: * {@code parallelism(p)} is already called. s/is already/has already been/ src/java.base/share/classes/javax/crypto/spec/Argon2ParameterSpec.java line 174: > 172: * {@code parallelism(p)} is already called. > 173: */ > 174: public Builder memoryKiB(int m) throws IllegalArgumentException { Don't need to declare IAE since it is a RuntimeException. src/java.base/share/classes/javax/crypto/spec/Argon2ParameterSpec.java line 183: > 181: * Set the memory value to the builder in power of two. > 182: * > 183: * @param mPower set memory value to 2^{@code mPower} Kibibytes remove "set". src/java.base/share/classes/javax/crypto/spec/Argon2ParameterSpec.java line 186: > 184: * @return this builder > 185: * @throws IllegalArgumentException > 186: * if {@code mPower} is less than 3 or larger than 30. > Or if I think it would read better with a semi-colon, ex: `if {@code mPower} is less than 3 or larger than 30; or if` src/java.base/share/classes/javax/crypto/spec/Argon2ParameterSpec.java line 187: > 185: * @throws IllegalArgumentException > 186: * if {@code mPower} is less than 3 or larger than 30. > Or if > 187: * {@code parallelism(p)} is already called, s/is already called/has already been called and/ src/java.base/share/classes/javax/crypto/spec/Argon2ParameterSpec.java line 191: > 189: */ > 190: public Builder memoryPowerOfTwo(int mPower) > 191: throws IllegalArgumentException { Don't need to declare IAE since it is a RuntimeException. src/java.base/share/classes/javax/crypto/spec/Argon2ParameterSpec.java line 385: > 383: > 384: /** > 385: * {@return the password bytes} Should say returns a new copy/clone each time it is called. Need to add an `@throws IllegalStateException`. src/java.base/share/classes/javax/crypto/spec/Argon2ParameterSpec.java line 395: > 393: > 394: /** > 395: * {@return the salt} Should say returns a new copy/clone each time it is called. ------------- PR Review: https://git.openjdk.org/jdk/pull/29597#pullrequestreview-4212713391 PR Review Comment: https://git.openjdk.org/jdk/pull/29597#discussion_r3174587177 PR Review Comment: https://git.openjdk.org/jdk/pull/29597#discussion_r3174593474 PR Review Comment: https://git.openjdk.org/jdk/pull/29597#discussion_r3174746065 PR Review Comment: https://git.openjdk.org/jdk/pull/29597#discussion_r3174824626 PR Review Comment: https://git.openjdk.org/jdk/pull/29597#discussion_r3174760401 PR Review Comment: https://git.openjdk.org/jdk/pull/29597#discussion_r3174763213 PR Review Comment: https://git.openjdk.org/jdk/pull/29597#discussion_r3174766239 PR Review Comment: https://git.openjdk.org/jdk/pull/29597#discussion_r3174814069 PR Review Comment: https://git.openjdk.org/jdk/pull/29597#discussion_r3174811757 PR Review Comment: https://git.openjdk.org/jdk/pull/29597#discussion_r3174808226 PR Review Comment: https://git.openjdk.org/jdk/pull/29597#discussion_r3174689032 PR Review Comment: https://git.openjdk.org/jdk/pull/29597#discussion_r3174718039
