Paul Eggert wrote: > > - if (grouplist && grouplist[n] == grp->gr_gid) > > + if (maxcount && grouplist[n] == grp->gr_gid) > > That change isn't needed for correctness, though, right? And the "-" line > looks a bit more efficient it's likely to need fewer registers.
When you start talking about efficiency, I wonder why the grouplist != NULL test is actually present inside the loop. The git history shows that it was added in commit 94076e90071e99e49b2bc751a86a9d330dd16242 on 1995-05-15, as a fix for a NULL access when the function is called with arguments maxcount == 0, grouplist == NULL, gid != -1. Which leads to the question: When the function is specified to "return the number of IDs we've written into GROUPLIST", why does it not return 0 in this case right away? The answer is that the caller (in lib/mgetgroups.c) expects the return value to be the needed size of the array (like in the Windows APIs that Sergey mentioned). Fixed as follows: 2026-08-21 Bruno Haible <[email protected]> getugroups: Clarify and optimize. * lib/getugroups.c (getugroups): Fix specification of return value. Move grouplist != NULL test out of the inner loop. * modules/getugroups (Depends-on): Add bool. diff --git a/lib/getugroups.c b/lib/getugroups.c index 6acb583fd9..6c86777046 100644 --- a/lib/getugroups.c +++ b/lib/getugroups.c @@ -52,8 +52,12 @@ getugroups (_GL_UNUSED int maxcount, process. Store at most MAXCOUNT group IDs in the GROUPLIST array. If GID is not -1, store it first (if possible). GID should be the group ID (pw_gid) obtained from getpwuid, in case USERNAME is not - listed in /etc/groups. Upon failure, set errno and return -1. - Otherwise, return the number of IDs we've written into GROUPLIST. */ + listed in /etc/groups. + Upon failure, set errno and return -1. Otherwise: + If MAXCOUNT is positive, return the number of IDs we've written into + GROUPLIST (necessarily <= MAXCOUNT). + If MAXCOUNT is zero, return the number of IDs we would have written + if GROUPLIST were non-NULL and MAXCOUNT were large enough. */ int getugroups (int maxcount, gid_t *grouplist, char const *username, @@ -81,13 +85,19 @@ getugroups (int maxcount, gid_t *grouplist, char const *username, if (streq (username, *cp)) { /* See if this group number is already on the list. */ - int n; - for (n = 0; n < count; ++n) - if (grouplist && grouplist[n] == grp->gr_gid) - break; + bool already_in_grouplist = false; + if (maxcount != 0) + { + for (int n = 0; n < count; ++n) + if (grouplist[n] == grp->gr_gid) + { + already_in_grouplist = true; + break; + } + } /* If it's a new group number, then try to add it to the list. */ - if (n == count) + if (!already_in_grouplist) { if (maxcount != 0) { diff --git a/modules/getugroups b/modules/getugroups index 0072fac171..af9b3ab614 100644 --- a/modules/getugroups +++ b/modules/getugroups @@ -9,6 +9,7 @@ m4/getugroups.m4 Depends-on: errno-h streq +bool configure.ac: gl_GETUGROUPS
