On Fri, 19 Dec 2025 17:56:23 GMT, Weijun Wang <[email protected]> wrote:
> Rewrite the native calls with FFM.
BTW, I don't believe that this change warrants new benchmarks.
src/jdk.security.auth/share/classes/com/sun/security/auth/module/UnixSystem.java
line 64:
> 62: int groupnum = getgroups(0, MemorySegment.NULL);
> 63: if (groupnum == -1) {
> 64: throw new RuntimeException("getgroups returns " +
> groupnum);
Not a must-have, but would be nice to have a FFM for `errno` to provide better
diagnostics.
src/jdk.security.auth/share/classes/com/sun/security/auth/module/UnixSystem.java
line 67:
> 65: }
> 66:
> 67: var gs = scope.allocate(gid_t, groupnum);
Should this be the size of gid_t * groupnum as the first argument and the byte
alignment as the second?
src/jdk.security.auth/share/classes/com/sun/security/auth/module/UnixSystem.java
line 70:
> 68: groupnum = getgroups(groupnum, gs);
> 69: if (groupnum == -1) {
> 70: throw new RuntimeException("getgroups returns " +
> groupnum);
Not a must-have, but would be nice to have a FFM for `errno` to provide better
diagnostics.
src/jdk.security.auth/share/classes/com/sun/security/auth/module/UnixSystem.java
line 78:
> 76: }
> 77:
> 78: var resbuf = passwd.allocate(scope);
Wouldn't this be 1024 as well?
src/jdk.security.auth/share/classes/com/sun/security/auth/module/UnixSystem.java
line 81:
> 79: var pwd = scope.allocate(C_POINTER);
> 80: var pwd_buf = scope.allocate(1024);
> 81: int out = getpwuid_r(getuid(), resbuf, pwd_buf,
> pwd_buf.byteSize(), pwd);
nit: resbuf is usually name pwd, which is used to reference the passwd
attributes in pwd_buf.
src/jdk.security.auth/share/classes/com/sun/security/auth/module/UnixSystem.java
line 93:
> 91: } catch (Throwable t) {
> 92: var error = new UnsatisfiedLinkError("FFM calls failed");
> 93: error.initCause(t);
Do you want to check with getCause() here as well?
src/jdk.security.auth/share/classes/jdk/internal/ffi/generated/jaas_unix/package-info.java
line 35:
> 33: * echo "#include <spwd.h>" >> $HEADER_NAME
> 34: *
> 35: *
nit: extra line, but please ignore if jextract generated.
-------------
PR Review: https://git.openjdk.org/jdk/pull/28931#pullrequestreview-3602940305
PR Review Comment: https://git.openjdk.org/jdk/pull/28931#discussion_r2638911674
PR Review Comment: https://git.openjdk.org/jdk/pull/28931#discussion_r2638911796
PR Review Comment: https://git.openjdk.org/jdk/pull/28931#discussion_r2638911943
PR Review Comment: https://git.openjdk.org/jdk/pull/28931#discussion_r2638912061
PR Review Comment: https://git.openjdk.org/jdk/pull/28931#discussion_r2638912196
PR Review Comment: https://git.openjdk.org/jdk/pull/28931#discussion_r2638912333
PR Review Comment: https://git.openjdk.org/jdk/pull/28931#discussion_r2638913897
