On Wed, 28 Aug 2024 13:24:04 GMT, Weijun Wang <wei...@openjdk.org> wrote:
>> Ferenc Rakoczi has updated the pull request incrementally with one
>> additional commit since the last revision:
>>
>> Code style changes suggested by Andrey Turbanov
>
> src/java.base/share/classes/sun/security/provider/SHA3.java line 74:
>
>> 72: private final byte suffix;
>> 73:
>> 74: // the stae matrix flattened into an array
>
> Typo: `stat` should be `state`.
Fixed.
> src/java.base/share/classes/sun/security/provider/SHA3.java line 84:
>
>> 82: // calls) will set it to 0 at its start.
>> 83: // When a squeeze() call uses up all available bytes from this state
>> 84: // and so a new keccak() call is made, squeezeOffset is reset to 0.
>
> The paragraph only mentions `set to 0` and `reset to 0`, and has not talked
> about other values. Maybe you can say when the squeeze call returns, it's set
> to a position that states before it have been used.
Well, the comment starts with "The byte offset in the state where the next
squeeze() will start."
> src/java.base/share/classes/sun/security/provider/SHA3.java line 136:
>
>> 134: */
>> 135: void implDigest(byte[] out, int ofs) {
>> 136: // Moving this allocation to the block where t is used causes a
>> little
>
> Typo, `t` should be `it`.
Fixed.
> src/java.base/share/classes/sun/security/provider/SHA3.java line 170:
>
>> 168:
>> 169: void implSqueeze(byte[]output, int offset, int numBytes) {
>> 170: // Moving this allocation to the block where t is used causes a
>> little
>
> Typo. `t` should be `it`.
Fixed.
> src/java.base/share/classes/sun/security/provider/SHA3.java line 225:
>
>> 223: ((squeezeOffset + numBytes - longOffset * 8) + 7) / 8;
>> 224:
>> 225: int limit = longOffset + longsToConvert - (numBytes % 8 == 0 ?
>> 0 : 1);
>
> Can we also do the same simplification here as in `implDigest`?
>
> int numLongs = numBytes / 8;
> for (int i = longOffset; i < longOffset + numLongs; i++) {
>
> After the `if` block above, `squeezeOffset` should be exactly `longOffset *
> 8`.
Fixed as suggested.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/20631#discussion_r1735008603
PR Review Comment: https://git.openjdk.org/jdk/pull/20631#discussion_r1735008691
PR Review Comment: https://git.openjdk.org/jdk/pull/20631#discussion_r1735008796
PR Review Comment: https://git.openjdk.org/jdk/pull/20631#discussion_r1735008980
PR Review Comment: https://git.openjdk.org/jdk/pull/20631#discussion_r1735009153
