On Tue, 14 Feb 2023 19:30:19 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 incrementally with three 
> additional commits since the last revision:
> 
>  - Another fixup.
>  - Correct comment in imageFile.hpp
>  - Copyright update in PerfectHashBuilder

LGTM

src/java.base/share/native/libjimage/imageFile.hpp line 217:

> 215: //
> 216: // Notes:
> 217: //  - Even though ATTRIBUTE_END (which might be encoded with a zero 
> byte) is used to

good

-------------

Marked as reviewed by jlaskey (Reviewer).

PR: https://git.openjdk.org/jdk/pull/12533

Reply via email to