On Thu, 9 Jan 2025 17:32:39 GMT, Severin Gehwolf <sgehw...@openjdk.org> wrote:
> During code review of > [JDK-8346239](https://bugs.openjdk.org/browse/JDK-8346239) a few comments > were made after the PR integrated. This follow-up patch cleans this up and > adds a unit test for the `JimageDiffGenerator` class. > > Testing: > - [ ] GHA > - [x] tools/jlink tests (including the new one). > > Thoughts? 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. 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? ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/23014#discussion_r1909225813 PR Review Comment: https://git.openjdk.org/jdk/pull/23014#discussion_r1909242794
