getgroups: fix logic error

The replacement getgroups mistakenly failed with EINVAL if there
were more than 20 groups, since -1 < n_groups.  Also, realloc
geometrically rather than linearly.

* lib/getgroups.c (rpl_getgroups): Don't fail if current process
has more than 20 groups.
* modules/getgroups-tests: New test.
* tests/test-getgroups.c: New file.

Signed-off-by: Eric Blake <ebb9@byu.net>

diff --git a/ChangeLog b/ChangeLog
index 0c6d6aa..099f1ee 100644 (file)
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,11 @@
+2009-11-13  Eric Blake  <ebb9@byu.net>
+
+       getgroups: fix logic error
+       * lib/getgroups.c (rpl_getgroups): Don't fail if current process
+       has more than 20 groups.
+       * modules/getgroups-tests: New test.
+       * tests/test-getgroups.c: New file.
+
 2009-11-13  Simon Josefsson  <simon@josefsson.org>
 
        * tests/test-base64.c: Improve.

diff --git a/lib/getgroups.c b/lib/getgroups.c
index 1e9cbc6..207a441 100644 (file)
--- a/lib/getgroups.c
+++ b/lib/getgroups.c
@@ -1,7 +1,7 @@
 /* provide consistent interface to getgroups for systems that don't allow N==0
 
-   Copyright (C) 1996, 1999, 2003, 2006, 2007, 2008 Free Software
-   Foundation, Inc.
+   Copyright (C) 1996, 1999, 2003, 2006, 2007, 2008, 2009 Free
+   Software Foundation, Inc.
 
    This program is free software: you can redistribute it and/or modify
    it under the terms of the GNU General Public License as published by
@@ -49,14 +49,14 @@ rpl_getgroups (int n, GETGROUPS_T *group)
   while (1)
     {
       /* No need to worry about address arithmetic overflow here,
-        since the ancient systems that we're running on have low
-        limits on the number of secondary groups.  */
+         since the ancient systems that we're running on have low
+         limits on the number of secondary groups.  */
       gbuf = xmalloc (n * sizeof *gbuf);
       n_groups = getgroups (n, gbuf);
-      if (n_groups < n)
-       break;
+      if (n_groups == -1 ? errno != EINVAL : n_groups < n)
+        break;
       free (gbuf);
-      n += 10;
+      n *= 2;
     }
 
   saved_errno = errno;

diff --git a/modules/getgroups-tests b/modules/getgroups-tests
new file mode 100644 (file)
index 0000000..f879e49
--- /dev/null
+++ b/modules/getgroups-tests
@@ -0,0 +1,12 @@
+Files:
+tests/test-getgroups.c
+
+Depends-on:
+progname
+
+configure.ac:
+
+Makefile.am:
+TESTS += test-getgroups
+check_PROGRAMS += test-getgroups
+test_getgroups_LDADD = $(LDADD) @LIBINTL@

diff --git a/tests/test-getgroups.c b/tests/test-getgroups.c
new file mode 100644 (file)
index 0000000..5324df1
--- /dev/null
+++ b/tests/test-getgroups.c
@@ -0,0 +1,84 @@
+/* Tests of getgroups.
+   Copyright (C) 2009 Free Software Foundation, Inc.
+
+   This program is free software: you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
+
+/* Written by Eric Blake <ebb9@byu.net>, 2009.  */
+
+#include <config.h>
+
+#include <unistd.h>
+
+#include <errno.h>
+#include <stdio.h>
+#include <stdlib.h>
+
+#define ASSERT(expr) \
+  do                                                                         \
+    {                                                                        \
+      if (!(expr))                                                           \
+        {                                                                    \
+          fprintf (stderr, "%s:%d: assertion failed\n", __FILE__, __LINE__);  \
+          fflush (stderr);                                                   \
+          abort ();                                                          \
+        }                                                                    \
+    }                                                                        \
+  while (0)
+
+int
+main (int argc, char **argv _UNUSED_PARAMETER_)
+{
+  int result;
+  GETGROUPS_T *groups;
+
+  errno = 0;
+  result = getgroups (0, NULL);
+  if (result == -1 && errno == ENOSYS)
+    {
+      fputs ("skipping test: no support for groups\n", stderr);
+      return 77;
+    }
+  ASSERT (0 <= result);
+  ASSERT (result + 1 < SIZE_MAX / sizeof *groups);
+  groups = malloc (result + 1 * sizeof *groups);
+  ASSERT (groups);
+  groups[result] = -1;
+  /* Check for EINVAL handling.  Not all processes have supplemental
+     groups, and getgroups does not have to return the effective gid,
+     so a result of 0 is reasonable.  Also, we can't test for EINVAL
+     if result is 1, because of how getgroups treats 0.  */
+  if (1 < result)
+    {
+      errno = 0;
+      ASSERT (getgroups (result - 1, groups) == -1);
+      ASSERT (errno == EINVAL);
+    }
+  ASSERT (getgroups (result, groups) == result);
+  ASSERT (getgroups (result + 1, groups) == result);
+  ASSERT (groups[result] == -1);
+  errno = 0;
+  ASSERT (getgroups (-1, NULL) == -1);
+  ASSERT (errno == EINVAL);
+
+  /* The automated unit test, with no arguments, ends here.  However,
+     for debugging purposes, you can pass a command-line argument to
+     list the returned groups.  */
+  if (1 < argc)
+    {
+      int i;
+      for (i = 0; i < result; i++)
+        printf ("%d\n", (int) groups[i]);
+    }
+  return 0;
+}