On Thu, 4 Jun 2026 08:15:29 GMT, Per Minborg <[email protected]> wrote:
>> Weijun Wang 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 six additional
>> commits since the last revision:
>>
>> - Merge branch 'master' into 8374154
>> - preserve old getTextSid; better error handling
>> - 8374154: Rewrite JAAS NTLoginModule with FFM
>> - revert three accidentally modified files
>> - Simpler GLE-methods that display error texts inside it; LocalFree
>> - the fix
>
> src/jdk.security.auth/share/classes/com/sun/security/auth/module/NTSystem.java
> line 63:
>
>> 61: .varHandle(groupElement("GetLastError"));
>> 62:
>> 63: private static void DisplayErrorText(Arena scope, String label,
>> MemorySegment cs) {
>
> Why is this method named with an initial capital letter (seems C# style
> rather than Java style)?
Will change. I must have read too many Win32 APIs while writing this.
> src/jdk.security.auth/share/classes/com/sun/security/auth/module/NTSystem.java
> line 109:
>
>> 107:
>> 108: private int OpenThreadTokenGLE(MemorySegment ThreadHandle, int
>> DesiredAccess, int OpenAsSelf, MemorySegment TokenHandle) {
>> 109: try (var arena = POOL.pushFrame(CAPTURE_LAYOUT.byteSize() + 64,
>
> Can we define the magic number 64 in a constant?
It was a heuristic for extra allocations that might be used by
`DisplayErrorText`.
Now I think I should use `POOL.pushFrame(CAPTURE_LAYOUT)` directly and let
`DisplayErrorText` managing its arena. After all, `debug` is not always turned
on.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/30629#discussion_r3356819175
PR Review Comment: https://git.openjdk.org/jdk/pull/30629#discussion_r3356816731
