On Tue, 14 Feb 2023 14:46:58 GMT, Severin Gehwolf <sgehw...@openjdk.org> wrote:
>> Could I please get a review of this trivial comment-only change? >> `imageFile.hpp` >> describes some properties of the jimage file `lib/modules`. However, I don't >> think >> the comment example matches current code in the JDK. >> [`ATTRIBUTE_OFFSET`](https://github.com/openjdk/jdk/blob/master/src/java.base/share/classes/jdk/internal/jimage/ImageLocation.java#L44) >> get >> written to the image file with value `0x05` while the comment mentions it >> gets >> written as `0x04`. I propose to fix the comment so that it matches the code. >> >> In passing, I've also fixed a comment related to `ATTRIBUTE_END`. I think >> the point >> being made there is about byte values of `[0x01..0x07]`, all would represent >> `ATTRIBUTE_END`, as the upper `5` bits would be `0`. Therefore, byte `0x01` >> would equally >> represent `ATTRIBUTE_END` as would `0x00` and `0x07` or any value in between. >> >> Thoughts? > > Severin Gehwolf has updated the pull request with a new target base due to a > merge or a rebase. The incremental webrev excludes the unrelated changes > brought in by the merge/rebase. The pull request contains three additional > commits since the last revision: > > - Merge branch 'master' into jdk-8302325-imageFile > - Copyright dates updated. > - 8302325: Wrong comment in java.base/share/native/libjimage/imageFile.hpp Changes requested by jlaskey (Reviewer). src/java.base/share/native/libjimage/imageFile.hpp line 218: > 216: // Notes: > 217: // - Even though ATTRIBUTE_END is used to mark the end of the attribute > stream, > 218: // streams will contain non-zero byte values to represent lesser > significant bits. This change is not correct. Maybe it is badly worded but the point is that **zeroes can** occur in the stream so testing for zero is insufficient. src/jdk.jlink/share/classes/jdk/tools/jlink/internal/PerfectHashBuilder.java line 249: > 247: // Attempt to pack entries until no collisions occur. > 248: if (!collidedEntries(bucket, count)) { > 249: // Failed to pack. Need to grow table. Change copyright date to mark this change. ------------- PR: https://git.openjdk.org/jdk/pull/12533
