On Mon, 24 Apr 2023 17:53:09 GMT, Anthony Scarpino <[email protected]>
wrote:
>> Weijun Wang has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> fine tuning spec
>
> src/java.base/share/classes/com/sun/crypto/provider/DHKEM.java line 175:
>
>> 173: this.keyAlgorithm = keyAlgorithm;
>> 174: this.hkdfAlgorithm = hkdfAlgorithm;
>> 175: suiteId = concat("KEM".getBytes(StandardCharsets.UTF_8),
>
> This is a general comment for all the `getBytes()` calls. I think these
> should be static final values. Each one of these usages is allocating a new
> String and byte[] every time.
OK.
> src/java.base/share/classes/com/sun/crypto/provider/DHKEM.java line 346:
>
>> 344: private Params paramsFromKey(Key k) throws InvalidKeyException {
>> 345: if (k instanceof ECKey eckey) {
>> 346: if (ECUtil.equals(eckey.getParams(),
>> CurveDB.lookup("secp256r1"))) {
>
> These lookup calls look like they could be static final values
Maybe I can define the static final values inside `CurveDB`.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/13256#discussion_r1175719339
PR Review Comment: https://git.openjdk.org/jdk/pull/13256#discussion_r1175720421
