On Tue, 6 May 2025 17:27:09 GMT, Jorn Vernee <jver...@openjdk.org> wrote:
>> Chen Liang has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> Update
>> src/java.base/share/classes/jdk/internal/foreign/abi/CapturableState.java
>>
>> Co-authored-by: Shaojin Wen <shaojin.we...@alibaba-inc.com>
>
> src/java.base/share/classes/jdk/internal/foreign/abi/CapturableState.java
> line 55:
>
>> 53: } else {
>> 54: supported = List.of(new CapturableState("errno", JAVA_INT, 1
>> << 2));
>> 55: }
>
> Maybe just split the initialization of `LAYOUT` and `LOOKUP` across these 2
> branches, instead of collecting everything into intermediate arrays. i.e.
>
> Suggestion:
>
> if (OperatingSystem.isWindows()) {
> LAYOUT = MemoryLayout.structLayout(
> JAVA_INT, // GetLastError
> JAVA_INT, // WSAGetLastError
> JAVA_INT // errno
> );
> LOOKUP = Map.of(
> "GetLastError", 1 << 0,
> "WSAGetLastError", 1 << 1,
> "errno", 1 << 2
> );
> } else {
> LAYOUT = MemoryLayout.structLayout(
> JAVA_INT // errno
> );
> LOOKUP = Map.of(
> "errno", 1 << 2
> );
> }
Then we can just make `CapturableState` a utility class. Should we proceed in
this direction?
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/25025#discussion_r2075969405
