2008-02-18 James Youngman <[EMAIL PROTECTED]>
* gl/m4/mgetgroups.m4: Check for getgrouplist.
* gl/lib/mgetgroups.c (mgetgroups): Use getgrouplist, if
available.
* TODO: Remove the item about switching to getgrouplist.
Signed-off-by: James Youngman <[EMAIL PROTECTED]>
---
TODO | 11 --------
gl/lib/mgetgroups.c | 70 ++++++++++++++++++++++++++++++++++++++++++++------
gl/m4/mgetgroups.m4 | 5 ++-
3 files changed, 64 insertions(+), 22 deletions(-)
diff --git a/TODO b/TODO
index 8c6b6fc..ae40efe 100644
--- a/TODO
+++ b/TODO
@@ -146,17 +146,6 @@ Add a distcheck-time test to ensure that every distributed
file is either read-only(indicating generated) or is
version-controlled and up to date.
-Implement Ulrich Drepper's suggestion to use getgrouplist rather than
- getugroups. This affects both `id' and `setuidgid', but makes a big
- difference on systems with many users and/or groups, and makes id usable
- once again on systems where access restrictions make getugroups fail.
- But first we'll need a run-test (either in an autoconf macro or at
- run time) to avoid the segfault bug in libc-2.3.2's getgrouplist.
- In that case, we'd revert to using a new (to-be-written) getgrouplist
- module that does most of what `id' already does. Or just avoid the
- buggy use of getgrouplist by never passing it a buffer of length zero.
- See http://bugzilla.redhat.com/200327
-
remove `%s' notation (now that they're all gone, add a Makefile.maint sc_
rule to ensure no new ones are added):
grep -E "\`%.{,4}s'" src/*.c
diff --git a/gl/lib/mgetgroups.c b/gl/lib/mgetgroups.c
index 6a4a422..3da1f58 100644
--- a/gl/lib/mgetgroups.c
+++ b/gl/lib/mgetgroups.c
@@ -23,11 +23,29 @@
#include <unistd.h>
#include <stdint.h>
+#include <string.h>
#include <errno.h>
-
+#if HAVE_GETGROUPLIST
+#include <grp.h>
+#endif
#include "getugroups.h"
#include "xalloc.h"
+
+static void*
+allocate_groupbuf (int size)
+{
+ if (xalloc_oversized (size, sizeof (GETGROUPS_T)))
+ {
+ errno = ENOMEM;
+ return NULL;
+ }
+ else
+ {
+ return malloc (size * sizeof (GETGROUPS_T));
+ }
+}
+
/* Like getugroups, but store the result in malloc'd storage.
Set *GROUPS to the malloc'd list of all group IDs of which USERNAME
is a member. If GID is not -1, store it first. GID should be the
@@ -43,6 +61,46 @@ mgetgroups (const char *username, gid_t gid, GETGROUPS_T
**groups)
int ng;
GETGROUPS_T *g;
+#if HAVE_GETGROUPLIST
+ /* We prefer to use getgrouplist if available, because it has better
+ * performance characteristics.
+ *
+ * In glibc 2.3.2, getgrouplist is buggy. If you pass a zero as the
+ * size of the output buffer, getgrouplist will still write to the
+ * buffer. Contrary to what some versions of the getgrouplist
+ * manpage say, this doesn't happen with nonzero buffer sizes.
+ * Therefore our usage here just avoids a zero sized buffer.
+ */
+ if (username)
+ {
+ enum { InitialGroupBufSize=1u };
+ /* InitialGroupBufSize is initially small to ensure good test coverage */
+ GETGROUPS_T smallbuf[InitialGroupBufSize];
+
+ max_n_groups = InitialGroupBufSize;
+ ng = getgrouplist (username, gid, smallbuf, &max_n_groups);
+
+ g = allocate_groupbuf (max_n_groups);
+ if (NULL == g)
+ return -1;
+
+ *groups = g;
+ if (max_n_groups > InitialGroupBufSize)
+ {
+ return getgrouplist (username, gid, g, &max_n_groups);
+ /* XXX: Ignoring the race with group size increase */
+ }
+ else
+ {
+ /* smallbuf was big enough, so we already have our data */
+ memcpy (g, smallbuf, max_n_groups * sizeof *g);
+ return 0;
+ }
+ /* getgrouplist failed, fall through and use getugroups instead. */
+ }
+ /* else no username, so fall through and use getgroups. */
+#endif
+
max_n_groups = (username
? getugroups (0, NULL, username, gid)
: getgroups (0, NULL));
@@ -52,14 +110,8 @@ mgetgroups (const char *username, gid_t gid, GETGROUPS_T
**groups)
if (max_n_groups < 0)
max_n_groups = 5;
- if (xalloc_oversized (max_n_groups, sizeof *g))
- {
- errno = ENOMEM;
- return -1;
- }
-
- g = malloc (max_n_groups * sizeof *g);
- if (g == NULL)
+ g = allocate_groupbuf (max_n_groups);
+ if (NULL == g)
return -1;
ng = (username
diff --git a/gl/m4/mgetgroups.m4 b/gl/m4/mgetgroups.m4
index 8183541..0cc9ae8 100644
--- a/gl/m4/mgetgroups.m4
+++ b/gl/m4/mgetgroups.m4
@@ -1,10 +1,11 @@
-#serial 1
-dnl Copyright (C) 2007 Free Software Foundation, Inc.
+#serial 2
+dnl Copyright (C) 2007,2008 Free Software Foundation, Inc.
dnl This file is free software; the Free Software Foundation
dnl gives unlimited permission to copy and/or distribute it,
dnl with or without modifications, as long as this notice is preserved.
AC_DEFUN([gl_MGETGROUPS],
[
+ AC_CHECK_FUNCS(getgrouplist)
AC_LIBOBJ([mgetgroups])
])
--
1.5.3.8
_______________________________________________
Bug-coreutils mailing list
Bug-coreutils@gnu.org
http://lists.gnu.org/mailman/listinfo/bug-coreutils
