On Thu, 26 Oct 2023 17:08:27 GMT, Sean Mullan <mul...@openjdk.org> wrote:
>> Weijun Wang has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> use @inheritDoc
>
> src/java.base/share/classes/java/security/interfaces/RSAPrivateKey.java line
> 74:
>
>> 72: */
>> 73: @Override
>> 74: default AlgorithmParameterSpec getParams() {
>
> What is the benefit of overriding this method if it returns the same type?
Otherwise, there will be an error `interface RSAPrivateKey inherits unrelated
defaults for getParams() from types AsymmetricKey and RSAKey`. I think this is
because `RSAKey` is not a `Key`.
> src/jdk.crypto.cryptoki/share/classes/sun/security/pkcs11/P11Key.java line
> 902:
>
>> 900: }
>> 901:
>> 902: public DSAParams getParams() {
>
> Suggest adding an `@Override` annotation here and below to make it more clear
> this is an overridden method.
There are a lot of places in this class without `@Override`. Shall I add all of
them or just the 2 places I touched this time?
> test/jdk/java/security/Signature/GetParams.java line 1:
>
>> 1: /*
>
> Why is this test in the Signature directory? Should it just be in the
> java/security dir?
It should not be in Signature. The java/security dir has only subdirs and no
test. Maybe I create a new AsymmetricKey dir and move it there?
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/16222#discussion_r1373502707
PR Review Comment: https://git.openjdk.org/jdk/pull/16222#discussion_r1373500436
PR Review Comment: https://git.openjdk.org/jdk/pull/16222#discussion_r1373505291
