Re: [PATCH] getugroups: Add _GL_ATTRIBUTE_NONNULL_IF_NONZERO for grouplist

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