On Fri, 17 Oct 2025 21:12:21 GMT, Valerie Peng <[email protected]> wrote:
>> Shawn M Emery has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> Updates for code review comments from @valeriepeng
>
> src/java.base/share/classes/com/sun/crypto/provider/AES_Crypt.java line 45:
>
>> 43: final class AES_Crypt extends SymmetricCipher {
>> 44:
>> 45: // Number of words in a block
>
> nit: from the usage, e.g. `int nk = key.length / WB`;, it seems WB means
> "number of bytes in a word".
I agree, it should be bytes per word for number of keys (nk) calculation, so
BW? I want to preserved words per block (WB) for maintainability (e.g., if we
decide to implement Rijndael-256, where WB = 8). Fixed.
> src/java.base/share/classes/com/sun/crypto/provider/AES_Crypt.java line 134:
>
>> 132: };
>> 133:
>> 134: private static final int[] T0 = {
>
> nit: add comment for all these precomputed lookup tables and their usage.
>
> Are these tables publicly available somewhere? I checked both spec in the
> class header and they don't have these included. I wonder if they are made
> available somewhere which corresponds with the current impl code better.
I generated the tables separately, but their usage is referenced in the
original specification cited in section 5.2.1. If made comments indicating of
such. Fixed.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/27807#discussion_r2441377285
PR Review Comment: https://git.openjdk.org/jdk/pull/27807#discussion_r2441377328
