Skip to content

[Android] Add shims for getgrnam_r and getgrgid_r for API 23 #1663

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
jmschonfeld merged 5 commits into from
Feb 11, 2026
Merged

[Android] Add shims for getgrnam_r and getgrgid_r for API 23 #1663

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
62ea75a
add shims
madsodgaard Dec 25, 2025
e30e7fb
fix errno and target platforms
madsodgaard Dec 25, 2025
0b13b30
PR feedback
madsodgaard Feb 7, 2026
0d5edb9
set passwd to NULL
madsodgaard Feb 9, 2026
87bb8a5
replace other targets by has_include
madsodgaard Feb 11, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions Sources/FoundationEssentials/Platform.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -164,7 +164,7 @@ extension Platform {
}

static func gid(forName name: String) -> gid_t? {
withUserGroupBuffer(name, group(), sizeProperty: Int32(_SC_GETGR_R_SIZE_MAX), operation: getgrnam_r) {
withUserGroupBuffer(name, group(), sizeProperty: Int32(_SC_GETGR_R_SIZE_MAX), operation: _filemanager_shims_getgrnam_r) {
$0.gr_gid
}
}
Expand Down Expand Up @@ -193,7 +193,7 @@ extension Platform {
}

static func name(forGID gid: gid_t) -> String? {
withUserGroupBuffer(gid, group(), sizeProperty: Int32(_SC_GETGR_R_SIZE_MAX), operation: getgrgid_r) {
withUserGroupBuffer(gid, group(), sizeProperty: Int32(_SC_GETGR_R_SIZE_MAX), operation: _filemanager_shims_getgrgid_r) {
// Android's gr_name `char *`` is nullable when it should be non-null.
// FIXME: avoid the coerce cast workaround once https://github.com/android/ndk/issues/2098 is fixed.
let gr_name: UnsafeMutablePointer<CChar>? = $0.gr_name
Expand Down
89 changes: 89 additions & 0 deletions Sources/_FoundationCShims/include/filemanager_shims.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
#define CSHIMS_FILEMANAGER_H

#include "_CShimsMacros.h"
#include "_CShimsTargetConditionals.h"

#if __has_include(<sys/param.h>)
#include <sys/param.h>
Expand Down Expand Up @@ -46,4 +47,92 @@
extern int _mkpath_np(const char *path, mode_t omode, const char **firstdir);
#endif

#if TARGET_OS_ANDROID && __ANDROID_API__ <= 23
#include <grp.h>
#include <sys/types.h>
#include <string.h>
#include <errno.h>

static inline int _filemanager_shims_getgrgid_r(gid_t gid, struct group *grp,
Copy link
Contributor

@itingliu itingliu Jan 13, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Any reason why we can't do call these code directly in Swift? Yes, there will be some boilerplates, but I'm not sure if we should be adding code to these

Copy link
Contributor

@jmschonfeld jmschonfeld Jan 13, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, in general this patch seems ok to me, but the goal of this project is to write these implementations in Swift rather than C, so unless we have a good reason to write this in C (because Swift cannot call these functions for some reason) then writing the shim in Swift seems like a better approach to me

Copy link
Member

@finagolfin finagolfin Jan 15, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How do you propose we write this in Swift? This was written in C because these APIs are only unavailable for Android API 23 and before, so either you'd have to mimic the #if TARGET_OS_ANDROID && __ANDROID_API__ <= 23 guard above with a compile-time API version check in Swift, which the language does not provide so we'd have to pass it in manually through the Package.swift manifest and CMake config, or use the new #available(Android, ) feature, which we can't yet because it requires NDK 28 or later.

I believe Mads added these as C shims for those reasons and because he noticed these C shims here already. What we could do is get this in for now, then revisit later in Swift once #available(Android, ) is working with a new NDK on the official Android CI.

Copy link
Member

@finagolfin finagolfin Jan 23, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would be good to get this small pull in, let me know what you think, @itingliu.

Copy link
Contributor

@itingliu itingliu Jan 23, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That sounds reasonable. Thanks

Copy link
Contributor

@itingliu itingliu Jan 23, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would you mind filing an issue for revisiting this to use #available(Android, ) when it's ready?

Copy link
Contributor Author

@madsodgaard madsodgaard Feb 21, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry for the delay, I filed it here #1769

char *buf, size_t buflen, struct group **result) {
errno = 0;

// Call the non-reentrant version.
// On Android, this uses Thread Local Storage (TLS),
// so it is safe from race conditions with other threads.
struct group *p = getgrgid(gid);

if (p == NULL) {
*result = NULL;
return errno;
}

if (strlcpy(buf, p->gr_name, buflen) >= buflen) {
*result = NULL;
return ERANGE;
}

grp->gr_name = buf;
grp->gr_gid = p->gr_gid;

// Android Bionic leaves this as NULL.
grp->gr_passwd = NULL;

// Android Bionic generates a synthetic list ["groupname", NULL].
// Replicating that here would require deep copying the strings array.
// Foundation does not use this either, so NULL is sufficient and avoids complexity.
grp->gr_mem = NULL;

*result = grp;
return 0;
}

static inline int _filemanager_shims_getgrnam_r(const char *name, struct group *grp,
char *buf, size_t buflen, struct group **result) {
errno = 0;

// Call the non-reentrant version.
// On Android, this uses Thread Local Storage (TLS),
// so it is safe from race conditions with other threads.
struct group *p = getgrnam(name);

if (p == NULL) {
*result = NULL;
return errno;
}

if (strlcpy(buf, p->gr_name, buflen) >= buflen) {
*result = NULL;
return ERANGE;
}

grp->gr_name = buf;
grp->gr_gid = p->gr_gid;

// Android Bionic leaves this as NULL.
grp->gr_passwd = NULL;

// Android Bionic generates a synthetic list ["groupname", NULL].
// Replicating that here would require deep copying the strings array.
// Foundation does not use this either, so NULL is sufficient and avoids complexity.
grp->gr_mem = NULL;

*result = grp;
return 0;
}

#elif __has_include(<grp.h>)
#include <grp.h>

static inline int _filemanager_shims_getgrgid_r(gid_t gid, struct group *grp,
char *buf, size_t buflen, struct group **result) {
return getgrgid_r(gid, grp, buf, buflen, result);
}

static inline int _filemanager_shims_getgrnam_r(const char *name, struct group *grp,
char *buf, size_t buflen, struct group **result) {
return getgrnam_r(name, grp, buf, buflen, result);
}
#endif

#endif // CSHIMS_FILEMANAGER_H