RFR: 8302325: Wrong comment in java.base/share/native/libjimage/imageFile.hpp

2023-02-13 Thread Severin Gehwolf
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/sh

Re: RFR: 8302325: Wrong comment in java.base/share/native/libjimage/imageFile.hpp

2023-02-14 Thread Alan Bateman
On Mon, 13 Feb 2023 14:12:15 GMT, Severin Gehwolf 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. > [`ATTRIB

Re: RFR: 8302325: Wrong comment in java.base/share/native/libjimage/imageFile.hpp

2023-02-14 Thread Severin Gehwolf
On Tue, 14 Feb 2023 10:40:31 GMT, Alan Bateman wrote: > Can you update the end date of the copyright headers as this is the first > change in 2023. imageFile.cpp by missed by the other change so it could be > done here too. Absolutely! Thanks for the review. - PR: https://git.ope

Re: RFR: 8302325: Wrong comment in java.base/share/native/libjimage/imageFile.hpp [v2]

2023-02-14 Thread Severin Gehwolf
> 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

Re: RFR: 8302325: Wrong comment in java.base/share/native/libjimage/imageFile.hpp [v2]

2023-02-14 Thread Severin Gehwolf
On Tue, 14 Feb 2023 10:40:31 GMT, Alan Bateman wrote: >> 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 >> com

Re: RFR: 8302325: Wrong comment in java.base/share/native/libjimage/imageFile.hpp [v2]

2023-02-14 Thread Alan Bateman
On Tue, 14 Feb 2023 10:40:31 GMT, Alan Bateman wrote: >> 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 >> com

Re: RFR: 8302325: Wrong comment in java.base/share/native/libjimage/imageFile.hpp [v2]

2023-02-14 Thread Jim Laskey
On Tue, 14 Feb 2023 14:46:58 GMT, Severin Gehwolf 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. >> [`

Re: RFR: 8302325: Wrong comment in java.base/share/native/libjimage/imageFile.hpp [v2]

2023-02-14 Thread Severin Gehwolf
On Tue, 14 Feb 2023 16:47:15 GMT, Jim Laskey wrote: >> 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 >> commi

Re: RFR: 8302325: Wrong comment in java.base/share/native/libjimage/imageFile.hpp [v2]

2023-02-14 Thread Severin Gehwolf
On Tue, 14 Feb 2023 16:59:03 GMT, Severin Gehwolf wrote: >> 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

Re: RFR: 8302325: Wrong comment in java.base/share/native/libjimage/imageFile.hpp [v2]

2023-02-14 Thread Jim Laskey
On Tue, 14 Feb 2023 14:46:58 GMT, Severin Gehwolf 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. >> [`

Re: RFR: 8302325: Wrong comment in java.base/share/native/libjimage/imageFile.hpp [v2]

2023-02-14 Thread Jim Laskey
On Tue, 14 Feb 2023 17:10:32 GMT, Severin Gehwolf wrote: >> @JimLaskey OK. Perhaps we can be clearer what is meant here exactly. I was >> having a hard time deciphering this. It does say `stream will contain zero >> byte values to represent lesser significant bits`. **What** are "byte values >

Re: RFR: 8302325: Wrong comment in java.base/share/native/libjimage/imageFile.hpp [v2]

2023-02-14 Thread Severin Gehwolf
On Tue, 14 Feb 2023 17:17:53 GMT, Jim Laskey wrote: >> To me it sounded like it wanted to say: Since the `ATTRIBUTE_END` isn't a >> full byte (only 5 bits in a byte), it might be represented as a non-zero >> value. For example a byte containing `0x07` would equally be `ATTRIBUTE_END` >> as wou

Re: RFR: 8302325: Wrong comment in java.base/share/native/libjimage/imageFile.hpp [v2]

2023-02-14 Thread Jim Laskey
On Tue, 14 Feb 2023 18:01:44 GMT, Severin Gehwolf wrote: >> I meant that an attribute can have zeros in the non-header portion of the >> attribute data. > > Got it. How about we change this comment from: > > // - Even though ATTRIBUTE_END is used to mark the end of the attribute > stream, > /

Re: RFR: 8302325: Wrong comment in java.base/share/native/libjimage/imageFile.hpp [v2]

2023-02-14 Thread Severin Gehwolf
On Tue, 14 Feb 2023 17:18:00 GMT, Jim Laskey wrote: >> 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 >> commi

Re: RFR: 8302325: Wrong comment in java.base/share/native/libjimage/imageFile.hpp [v2]

2023-02-14 Thread Severin Gehwolf
On Tue, 14 Feb 2023 18:10:11 GMT, Jim Laskey wrote: >> Got it. How about we change this comment from: >> >> // - Even though ATTRIBUTE_END is used to mark the end of the attribute >> stream, >> // streams will contain zero byte values to represent lesser >> significant bits. >> // T

Re: RFR: 8302325: Wrong comment in java.base/share/native/libjimage/imageFile.hpp [v3]

2023-02-14 Thread Severin Gehwolf
> 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

Re: RFR: 8302325: Wrong comment in java.base/share/native/libjimage/imageFile.hpp [v3]

2023-02-14 Thread Jim Laskey
On Tue, 14 Feb 2023 19:30:19 GMT, Severin Gehwolf 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. >> [`

Re: RFR: 8302325: Wrong comment in java.base/share/native/libjimage/imageFile.hpp [v3]

2023-02-15 Thread Severin Gehwolf
On Tue, 14 Feb 2023 19:30:19 GMT, Severin Gehwolf 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. >> [`