On Tue, 6 May 2025 17:40:52 GMT, Chen Liang <li...@openjdk.org> wrote:

>> 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?

Yes, that seems fine to me.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/25025#discussion_r2076005578

Reply via email to