On Thu, 25 May 2023 20:23:19 GMT, Valerie Peng <valer...@openjdk.org> wrote:
>> Martin Balao has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> 8301553: Support Password-Based Cryptography in SunPKCS11 (iteration #3)
>>
>> Co-authored-by: Francisco Ferrari <fferr...@redhat.com>
>> Co-authored-by: Martin Balao <mba...@redhat.com>
>
> src/jdk.crypto.cryptoki/share/classes/sun/security/pkcs11/P11Util.java line
> 34:
>
>> 32: import java.nio.charset.Charset;
>> 33: import java.security.*;
>> 34: import java.util.Arrays;
>
> Not used?
Good
> src/jdk.crypto.cryptoki/share/classes/sun/security/pkcs11/P11Util.java line
> 36:
>
>> 34: import java.util.Arrays;
>> 35:
>> 36: import jdk.internal.access.SharedSecrets;
>
> Not used?
Good
> src/jdk.crypto.cryptoki/share/classes/sun/security/pkcs11/wrapper/CK_MECHANISM.java
> line 213:
>
>> 211: sb.append(Constants.INDENT);
>> 212: sb.append("pParameter:");
>> 213: sb.append(Constants.NEWLINE);
>
> Is this intended? It seems NEWLINE is not added beforehand for other fields.
We found it more clear to start in a new line when showing the inner
parameters. The reason is that inner parameters can be a structure (as in the
PBE case) which has its own "member-name:" and new lines. It looked a bit odd
when we had for example " pParameter: pInitVector:" in the first line and
the rest of the inner structure below.
> src/jdk.crypto.cryptoki/share/classes/sun/security/pkcs11/wrapper/PKCS11.java
> line 2:
>
>> 1: /*
>> 2: * Copyright (c) 2003, 2023, Oracle and/or its affiliates. All rights
>> reserved.
>
> No changes in this file, should not update the year?
Good point. There were changes at some point that were undone in one of the
iterations.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/12396#discussion_r1206095706
PR Review Comment: https://git.openjdk.org/jdk/pull/12396#discussion_r1206095604
PR Review Comment: https://git.openjdk.org/jdk/pull/12396#discussion_r1206098715
PR Review Comment: https://git.openjdk.org/jdk/pull/12396#discussion_r1206099551
