On Thu, 9 Jan 2025 17:49:23 GMT, Henry Jen <[email protected]> wrote:
>> Severin Gehwolf has updated the pull request incrementally with one
>> additional commit since the last revision:
>>
>> Review feedback
>
> src/jdk.jlink/share/classes/jdk/tools/jlink/internal/runtimelink/JimageDiffGenerator.java
> line 133:
>
>> 131: while ((bytesRead1 = is1.readNBytes(buf1, 0, buf1.length))
>> != 0) {
>> 132: bytesRead2 = is2.readNBytes(buf2, 0, buf2.length);
>> 133: if (bytesRead2 == 0) {
>
> This is redundant to next statement. However, we do need to read and test
> byteRead2 is also 0 after byteRead1 is 0 and exit the loop.
OK.
> test/jdk/tools/jlink/runtimeImage/JimageDiffGeneratorTest.java line 1:
>
>> 1: /*
>
> Looks good to me. Nitpick: Most test cases use a common base image can be
> defined once?
They are slightly different each. I don't think this will make the code a lot
shorter/easier to read. As it is it keeps the test cases independent as much as
possible.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/23014#discussion_r1909262375
PR Review Comment: https://git.openjdk.org/jdk/pull/23014#discussion_r1909262161