On Thu, 14 Nov 2024 16:20:22 GMT, Artur Barashev <abaras...@openjdk.org> wrote:
>> Volodymyr Paprotski has updated the pull request incrementally with one >> additional commit since the last revision: >> >> comments from Kevin > > src/java.base/share/classes/com/sun/crypto/provider/CipherBlockChaining.java > line 65: > >> 63: // Should be large enough to provide intrinsic with optimization >> 64: // opportunities >> 65: private final int chunkSize = 1024*100; > > Shouldn't we call `getBlockSize()` of the parent class here rather than using > a hard-coded integer? Variable assignment to be done in constructor in such > case. I don't think this constant needs to be dynamic. The reason I mention blocksize, the intrinsic expects multiple of block size (`ArrayUtil.blockSizeCheck(plainLen, blockSize);` assert before-hand), but its otherwise unrelated to the block size. (i.e. "has to be large enough for optimizations"). I believe this class can also be used by DES; iirc, blocksize=8. I suppose I could use `AESConstants.AES_BLOCK_SIZE*6400` as `chunkSize`. If that looks good, I will do that change, though I've seen '16' used as a raw number plenty of times in the package, without 'attribution' to AES_BLOCK_SIZE. And to be thorough, I should probably put an `ArrayUtil.blockSizeCheck(chunkSize, blockSize);` into the constructor? That way it is checked, but not on every call. ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/22086#discussion_r1844325933
