On Tue, 14 Jul 2026 16:46:54 GMT, Sean Mullan <[email protected]> wrote:
>> Valerie Peng has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> updated to address review comments.
>
> src/java.base/share/classes/com/sun/crypto/provider/Argon2DerivedKey.java
> line 48:
>
>> 46: * @since 27
>> 47: */
>> 48: public final class Argon2DerivedKey implements SecretKey, Destroyable {
>
> Don't need "implements Destroyable" since `SecretKey` already implements it.
Yes
> src/java.base/share/classes/com/sun/crypto/provider/Argon2DerivedKey.java
> line 61:
>
>> 59:
>> 60: /**
>> 61: * Create a Argon2 derived secret key using the supplied arguments.
>
> s/a/an/
Yes.
> src/java.base/share/classes/com/sun/crypto/provider/Argon2DerivedKey.java
> line 63:
>
>> 61: * Create a Argon2 derived secret key using the supplied arguments.
>> 62: *
>> 63: * @param type the Argon2 variant.
>
> Nit: no need for periods in `@param` and `@exception` descriptions.
Yes.
> src/java.base/share/classes/com/sun/crypto/provider/Argon2DerivedKey.java
> line 68:
>
>> 66: * @param algo the algorithm for the derived key.
>> 67: *
>> 68: * @exception InvalidKeyException if less than 8 bytes are available
>> for
>
> Remove? This ctor does not throw IKE.
Correct, will do.
> src/java.base/share/classes/com/sun/crypto/provider/Argon2DerivedKey.java
> line 68:
>
>> 66: * @param algo the algorithm for the derived key.
>> 67: *
>> 68: * @exception InvalidKeyException if less than 8 bytes are available
>> for
>
> `@throws` is more commonly used now than `@exception`.
n/a due to removal.
> src/java.base/share/classes/com/sun/crypto/provider/Argon2DerivedKey.java
> line 71:
>
>> 69: * the key.
>> 70: */
>> 71: public Argon2DerivedKey(String type, Argon2ParameterSpec spec,
>
> Can be package-private I think.
Yes.
> src/java.base/share/classes/com/sun/crypto/provider/Argon2DerivedKey.java
> line 80:
>
>> 78: }
>> 79:
>> 80: public byte[] getEncoded() {
>
> Add `@Override`.
Ok.
> src/java.base/share/classes/com/sun/crypto/provider/Argon2DerivedKey.java
> line 89:
>
>> 87: }
>> 88:
>> 89: public String getAlgorithm() {
>
> Add `@Override`.
Yes
> src/java.base/share/classes/com/sun/crypto/provider/Argon2DerivedKey.java
> line 93:
>
>> 91: }
>> 92:
>> 93: public String getFormat() {
>
> Add `@Override`.
Yes
> src/java.base/share/classes/com/sun/crypto/provider/Argon2DerivedKey.java
> line 174:
>
>> 172: *
>> 173: * @throws java.io.ObjectStreamException if a new object
>> representing
>> 174: * this DES key could not be created
>
> s/DES/Argon2id/
Yes, will fix.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/29597#discussion_r3708363969
PR Review Comment: https://git.openjdk.org/jdk/pull/29597#discussion_r3708371184
PR Review Comment: https://git.openjdk.org/jdk/pull/29597#discussion_r3708389486
PR Review Comment: https://git.openjdk.org/jdk/pull/29597#discussion_r3708379573
PR Review Comment: https://git.openjdk.org/jdk/pull/29597#discussion_r3708382160
PR Review Comment: https://git.openjdk.org/jdk/pull/29597#discussion_r3708368155
PR Review Comment: https://git.openjdk.org/jdk/pull/29597#discussion_r3708355081
PR Review Comment: https://git.openjdk.org/jdk/pull/29597#discussion_r3708355788
PR Review Comment: https://git.openjdk.org/jdk/pull/29597#discussion_r3708361268
PR Review Comment: https://git.openjdk.org/jdk/pull/29597#discussion_r3708385242