These days, I'm not sure how many systems still need the getgroups replacement because the system getgroups is buggy. However, there is the issue of mingw, which lacks getgroups, and for that matter, any notion of group management (well, windows does have groups, as evidenced by the fact that cygwin is able to manage them; but mingw does not have any way to access or manipulate groups: there is no get[e]gid, and stat.st_gid is always 0). At any rate, it's nicer to guarantee that mingw can at least link with getgroups, even if it does nothing.
The getgroups replacement had a logic bug, which made it fail if you had more than 20 supplemental groups. To prove this, I added a unit test, then tested with ac_cv_func_getgroups_works=no and an account that belongs to 46 groups. Meanwhile, autoconf declares GETGROUPS_T as either int or gid_t, but that type is probably obsolete since the number of modern porting systems that either don't declare getgroups or declare it with the wrong type is probably 0. But since we have the ability to do replacement functions, I'd rather have the _only_ use of GETGROUPS_T be in getgroups.c, rather than making everyone else have to use it. This is an API change, but only affects the rare platform where getgroups has the wrong type. getgroups replaces a library function, so it should not exit(). On the other hand, the mgetgroups interface from coreutils is much nicer to use (one call, instead of 2); that would be a reasonable place to add an xalloc-die dependency, but this patch does not do that. I will be posting a followup patch to the coreutils list for using this series. Any problems with committing this series? Eric Blake (5): getgroups: fix logic error getgroups: avoid calling exit getgroups: provide stub for mingw getgroups: don't expose GETGROUPS_T to user mgetgroups: new module, taken from coreutils For reference, after the patch I also posted the interdiff from coreutils' original mgetgroups implementation. >From c6fb527004db6251881626f1a34aea7abaf26f63 Mon Sep 17 00:00:00 2001 From: Eric Blake <e...@byu.net> Date: Thu, 12 Nov 2009 08:51:45 -0700 Subject: [PATCH 1/5] 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 <e...@byu.net> --- ChangeLog | 6 +++ lib/getgroups.c | 14 ++++---- modules/getgroups-tests | 12 +++++++ tests/test-getgroups.c | 84 +++++++++++++++++++++++++++++++++++++++++++++++ 4 files changed, 109 insertions(+), 7 deletions(-) create mode 100644 modules/getgroups-tests create mode 100644 tests/test-getgroups.c diff --git a/ChangeLog b/ChangeLog index 405bfda..b7f725b 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,5 +1,11 @@ 2009-11-12 Eric Blake <e...@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. + version-etc: match standards.texi style * lib/version-etc.c (emit_bug_reporting_address): Drop periods, and use <> only for URLs. diff --git a/lib/getgroups.c b/lib/getgroups.c index 1e9cbc6..207a441 100644 --- 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 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 index 0000000..da027d0 --- /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 <e...@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", groups[i]); + } + return 0; +} -- 1.6.4.2 >From 5f4116ebb23d1ed74cdbdd86cc9fbcc5d1315ea5 Mon Sep 17 00:00:00 2001 From: Eric Blake <e...@byu.net> Date: Thu, 12 Nov 2009 09:30:38 -0700 Subject: [PATCH 2/5] getgroups: avoid calling exit rpl_getgroups should be a library function, comparable to glibc. * modules/getgroups (Depends-on): Add malloc-posix and unistd, drop xalloc. * lib/getgroups.c (rpl_getgroups): Fail with ENOMEM rather than exiting, in the rare case of malloc failure. Signed-off-by: Eric Blake <e...@byu.net> --- ChangeLog | 6 ++++++ lib/getgroups.c | 21 +++++++++++---------- modules/getgroups | 6 +++--- 3 files changed, 20 insertions(+), 13 deletions(-) diff --git a/ChangeLog b/ChangeLog index b7f725b..ea96466 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,5 +1,11 @@ 2009-11-12 Eric Blake <e...@byu.net> + getgroups: avoid calling exit + * modules/getgroups (Depends-on): Add malloc-posix and unistd, + drop xalloc. + * lib/getgroups.c (rpl_getgroups): Fail with ENOMEM rather than + exiting, in the rare case of malloc failure. + getgroups: fix logic error * lib/getgroups.c (rpl_getgroups): Don't fail if current process has more than 20 groups. diff --git a/lib/getgroups.c b/lib/getgroups.c index 207a441..e764d73 100644 --- a/lib/getgroups.c +++ b/lib/getgroups.c @@ -20,20 +20,19 @@ #include <config.h> -#undef getgroups +#include <unistd.h> -#include <stdio.h> -#include <sys/types.h> #include <errno.h> #include <stdlib.h> -#include <unistd.h> -#include "xalloc.h" +#undef getgroups -/* On at least Ultrix 4.3 and NextStep 3.2, getgroups (0, 0) always fails. - On other systems, it returns the number of supplemental groups for the - process. This function handles that special case and lets the system- - provided function handle all others. */ +/* On at least Ultrix 4.3 and NextStep 3.2, getgroups (0, NULL) always + fails. On other systems, it returns the number of supplemental + groups for the process. This function handles that special case + and lets the system-provided function handle all others. However, + it can fail with ENOMEM if memory is tight. It is unspecified + whether the effective group id is included in the list. */ int rpl_getgroups (int n, GETGROUPS_T *group) @@ -51,7 +50,9 @@ rpl_getgroups (int n, GETGROUPS_T *group) /* 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. */ - gbuf = xmalloc (n * sizeof *gbuf); + gbuf = malloc (n * sizeof *gbuf); + if (!gbuf) + return -1; n_groups = getgroups (n, gbuf); if (n_groups == -1 ? errno != EINVAL : n_groups < n) break; diff --git a/modules/getgroups b/modules/getgroups index 16604f8..472d20e 100644 --- a/modules/getgroups +++ b/modules/getgroups @@ -6,7 +6,8 @@ lib/getgroups.c m4/getgroups.m4 Depends-on: -xalloc +malloc-posix +unistd configure.ac: gl_FUNC_GETGROUPS @@ -20,5 +21,4 @@ License: GPL Maintainer: -Jim Meyering - +Jim Meyering, Eric Blake -- 1.6.4.2 >From c5b8cc5a5f04e0fd5a1f68bc65cb0afce4d71227 Mon Sep 17 00:00:00 2001 From: Eric Blake <e...@byu.net> Date: Thu, 12 Nov 2009 09:53:14 -0700 Subject: [PATCH 3/5] getgroups: provide stub for mingw Avoid link failure on mingw, which lacks getgroups and anything else related to gid_t management (stat.st_gid is always 0). * lib/getgroups.c (getgroups): Provide ENOSYS stub for mingw. * m4/getgroups.m4 (gl_FUNC_GETGROUPS): Check for missing function. Modernize replacement scheme. (gl_PREREQ_GETGROUPS): Delete. * modules/getgroups (configure.ac): Declare witness. * m4/unistd_h.m4 (gl_UNISTD_H_DEFAULTS): Add default. * modules/unistd (Depends-on): Substitute witness. * lib/unistd.in.h (getgroups): Declare replacement. Signed-off-by: Eric Blake <e...@byu.net> --- ChangeLog | 10 ++++++++++ lib/getgroups.c | 17 ++++++++++++++++- lib/unistd.in.h | 22 ++++++++++++++++++++++ m4/getgroups.m4 | 19 ++++++++----------- m4/unistd_h.m4 | 5 ++++- modules/getgroups | 1 + modules/unistd | 3 +++ 7 files changed, 64 insertions(+), 13 deletions(-) diff --git a/ChangeLog b/ChangeLog index ea96466..968d79d 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,5 +1,15 @@ 2009-11-12 Eric Blake <e...@byu.net> + getgroups: provide stub for mingw + * lib/getgroups.c (getgroups): Provide ENOSYS stub for mingw. + * m4/getgroups.m4 (gl_FUNC_GETGROUPS): Check for missing + function. Modernize replacement scheme. + (gl_PREREQ_GETGROUPS): Delete. + * modules/getgroups (configure.ac): Declare witness. + * m4/unistd_h.m4 (gl_UNISTD_H_DEFAULTS): Add default. + * modules/unistd (Depends-on): Substitute witness. + * lib/unistd.in.h (getgroups): Declare replacement. + getgroups: avoid calling exit * modules/getgroups (Depends-on): Add malloc-posix and unistd, drop xalloc. diff --git a/lib/getgroups.c b/lib/getgroups.c index e764d73..bad676e 100644 --- a/lib/getgroups.c +++ b/lib/getgroups.c @@ -25,7 +25,20 @@ #include <errno.h> #include <stdlib.h> -#undef getgroups +#if !HAVE_GETGROUPS + +/* Provide a stub that fails with ENOSYS, since there is no group + information available on mingw. */ +int +getgroups (int n _UNUSED_PARAMETER_, GETGROUPS_T *groups _UNUSED_PARAMETER_) +{ + errno = ENOSYS; + return -1; +} + +#else /* HAVE_GETGROUPS */ + +# undef getgroups /* On at least Ultrix 4.3 and NextStep 3.2, getgroups (0, NULL) always fails. On other systems, it returns the number of supplemental @@ -66,3 +79,5 @@ rpl_getgroups (int n, GETGROUPS_T *group) return n_groups; } + +#endif /* HAVE_GETGROUPS */ diff --git a/lib/unistd.in.h b/lib/unistd.in.h index b6d2fa0..90494e4 100644 --- a/lib/unistd.in.h +++ b/lib/unistd.in.h @@ -406,6 +406,28 @@ extern int getdtablesize (void); #endif +#if @GNULIB_GETGROUPS@ +# if @REPLACE_GETGROUPS@ +# undef getgroups +# define getgroups rpl_getgroups +# endif +# if !...@have_getgroups@ || @REPLACE_GETGROUPS@ +/* Return the supplemental groups that the current process belongs to. + It is unspecified whether the effective group id is in the list. + If N is 0, return the group count; otherwise, N describes how many + entries are available in GROUPS. Return -1 and set errno if N is + not 0 and not large enough. Fails with ENOSYS on some systems. */ +int getgroups (int n, GETGROUPS_T *groups); +# endif +#elif defined GNULIB_POSIXCHECK +# undef getgroups +# define getgroups(n,g) \ + (GL_LINK_WARNING ("getgroups is unportable - " \ + "use gnulib module getgroups for portability"), \ + getgroups (n, g)) +#endif + + #if @GNULIB_GETHOSTNAME@ /* Return the standard host name of the machine. WARNING! The host name may or may not be fully qualified. diff --git a/m4/getgroups.m4 b/m4/getgroups.m4 index a7019fb..1dd39ea 100644 --- a/m4/getgroups.m4 +++ b/m4/getgroups.m4 @@ -1,4 +1,4 @@ -# serial 12 +# serial 13 dnl From Jim Meyering. dnl A wrapper around AC_FUNC_GETGROUPS. @@ -12,17 +12,14 @@ dnl A wrapper around AC_FUNC_GETGROUPS. AC_DEFUN([gl_FUNC_GETGROUPS], [ AC_REQUIRE([AC_FUNC_GETGROUPS]) - if test "$ac_cv_func_getgroups_works" != yes; then + AC_REQUIRE([AC_TYPE_GETGROUPS]) + AC_REQUIRE([gl_UNISTD_H_DEFAULTS]) + if test "$ac_cv_func_getgroups" != yes; then + AC_LIBOBJ([getgroups]) + HAVE_GETGROUPS=0 + elif test "$ac_cv_func_getgroups_works" != yes; then AC_LIBOBJ([getgroups]) - AC_DEFINE([getgroups], [rpl_getgroups], - [Define as rpl_getgroups if getgroups doesn't work right.]) - gl_PREREQ_GETGROUPS + REPLACE_GETGROUPS=1 fi test -n "$GETGROUPS_LIB" && LIBS="$GETGROUPS_LIB $LIBS" ]) - -# Prerequisites of lib/getgroups.c. -AC_DEFUN([gl_PREREQ_GETGROUPS], -[ - AC_REQUIRE([AC_TYPE_GETGROUPS]) -]) diff --git a/m4/unistd_h.m4 b/m4/unistd_h.m4 index 5aa39ae..5c94916 100644 --- a/m4/unistd_h.m4 +++ b/m4/unistd_h.m4 @@ -1,4 +1,4 @@ -# unistd_h.m4 serial 31 +# unistd_h.m4 serial 32 dnl Copyright (C) 2006-2009 Free Software Foundation, Inc. dnl This file is free software; the Free Software Foundation dnl gives unlimited permission to copy and/or distribute it, @@ -46,6 +46,7 @@ AC_DEFUN([gl_UNISTD_H_DEFAULTS], GNULIB_GETCWD=0; AC_SUBST([GNULIB_GETCWD]) GNULIB_GETDOMAINNAME=0; AC_SUBST([GNULIB_GETDOMAINNAME]) GNULIB_GETDTABLESIZE=0; AC_SUBST([GNULIB_GETDTABLESIZE]) + GNULIB_GETGROUPS=0; AC_SUBST([GNULIB_GETGROUPS]) GNULIB_GETHOSTNAME=0; AC_SUBST([GNULIB_GETHOSTNAME]) GNULIB_GETLOGIN_R=0; AC_SUBST([GNULIB_GETLOGIN_R]) GNULIB_GETPAGESIZE=0; AC_SUBST([GNULIB_GETPAGESIZE]) @@ -76,6 +77,7 @@ AC_DEFUN([gl_UNISTD_H_DEFAULTS], HAVE_FTRUNCATE=1; AC_SUBST([HAVE_FTRUNCATE]) HAVE_GETDOMAINNAME=1; AC_SUBST([HAVE_GETDOMAINNAME]) HAVE_GETDTABLESIZE=1; AC_SUBST([HAVE_GETDTABLESIZE]) + HAVE_GETGROUPS=1; AC_SUBST([HAVE_GETGROUPS]) HAVE_GETHOSTNAME=1; AC_SUBST([HAVE_GETHOSTNAME]) HAVE_GETPAGESIZE=1; AC_SUBST([HAVE_GETPAGESIZE]) HAVE_GETUSERSHELL=1; AC_SUBST([HAVE_GETUSERSHELL]) @@ -99,6 +101,7 @@ AC_DEFUN([gl_UNISTD_H_DEFAULTS], REPLACE_FCHDIR=0; AC_SUBST([REPLACE_FCHDIR]) REPLACE_FCHOWNAT=0; AC_SUBST([REPLACE_FCHOWNAT]) REPLACE_GETCWD=0; AC_SUBST([REPLACE_GETCWD]) + REPLACE_GETGROUPS=0; AC_SUBST([REPLACE_GETGROUPS]) REPLACE_GETPAGESIZE=0; AC_SUBST([REPLACE_GETPAGESIZE]) REPLACE_LCHOWN=0; AC_SUBST([REPLACE_LCHOWN]) REPLACE_LINK=0; AC_SUBST([REPLACE_LINK]) diff --git a/modules/getgroups b/modules/getgroups index 472d20e..8ecbbdd 100644 --- a/modules/getgroups +++ b/modules/getgroups @@ -11,6 +11,7 @@ unistd configure.ac: gl_FUNC_GETGROUPS +gl_UNISTD_MODULE_INDICATOR([getgroups]) Makefile.am: diff --git a/modules/unistd b/modules/unistd index d299e4a..48259ac 100644 --- a/modules/unistd +++ b/modules/unistd @@ -39,6 +39,7 @@ unistd.h: unistd.in.h -e 's|@''GNULIB_GETCWD''@|$(GNULIB_GETCWD)|g' \ -e 's|@''GNULIB_GETDOMAINNAME''@|$(GNULIB_GETDOMAINNAME)|g' \ -e 's|@''GNULIB_GETDTABLESIZE''@|$(GNULIB_GETDTABLESIZE)|g' \ + -e 's|@''GNULIB_GETGROUPS''@|$(GNULIB_GETGROUPS)|g' \ -e 's|@''GNULIB_GETHOSTNAME''@|$(GNULIB_GETHOSTNAME)|g' \ -e 's|@''GNULIB_GETLOGIN_R''@|$(GNULIB_GETLOGIN_R)|g' \ -e 's|@''GNULIB_GETPAGESIZE''@|$(GNULIB_GETPAGESIZE)|g' \ @@ -68,6 +69,7 @@ unistd.h: unistd.in.h -e 's|@''HAVE_FTRUNCATE''@|$(HAVE_FTRUNCATE)|g' \ -e 's|@''HAVE_GETDOMAINNAME''@|$(HAVE_GETDOMAINNAME)|g' \ -e 's|@''HAVE_GETDTABLESIZE''@|$(HAVE_GETDTABLESIZE)|g' \ + -e 's|@''HAVE_GETGROUPS''@|$(HAVE_GETGROUPS)|g' \ -e 's|@''HAVE_GETHOSTNAME''@|$(HAVE_GETHOSTNAME)|g' \ -e 's|@''HAVE_GETPAGESIZE''@|$(HAVE_GETPAGESIZE)|g' \ -e 's|@''HAVE_GETUSERSHELL''@|$(HAVE_GETUSERSHELL)|g' \ @@ -91,6 +93,7 @@ unistd.h: unistd.in.h -e 's|@''REPLACE_FCHDIR''@|$(REPLACE_FCHDIR)|g' \ -e 's|@''REPLACE_FCHOWNAT''@|$(REPLACE_FCHOWNAT)|g' \ -e 's|@''REPLACE_GETCWD''@|$(REPLACE_GETCWD)|g' \ + -e 's|@''REPLACE_GETGROUPS''@|$(REPLACE_GETGROUPS)|g' \ -e 's|@''REPLACE_GETPAGESIZE''@|$(REPLACE_GETPAGESIZE)|g' \ -e 's|@''REPLACE_LCHOWN''@|$(REPLACE_LCHOWN)|g' \ -e 's|@''REPLACE_LINK''@|$(REPLACE_LINK)|g' \ -- 1.6.4.2 >From d7b3e62d1228f7e55d9e9d9cfde5aa5982f7c60e Mon Sep 17 00:00:00 2001 From: Eric Blake <e...@byu.net> Date: Thu, 12 Nov 2009 10:19:39 -0700 Subject: [PATCH 4/5] getgroups: don't expose GETGROUPS_T to user These days, most systems already declare getgroups with gid_t*. But in the rare case that GETGROUPS_T is still int but gid_t is short, the user should not have to uglify their code; let the replacement hide all the magic. Tested by configuring with ac_cv_type_getgroups=uint64_t on a platform with 32-bit gid_t, and ignoring compiler warnings. * lib/getgroups.c (rpl_getgroups): Change signature. Copy array an element at a time if GETGROUPS_T is wrong size. * lib/getugroups.h (getugroups): Change signature. * lib/unistd.in.h (getgroups): Likewise. * m4/getgroups.m4 (gl_FUNC_GETGROUPS): Use replacement if signature needs fixing. * m4/getugroups.m4 (gl_GETUGROUPS): No longer need AC_TYPE_GETGROUPS. * modules/group-member (Depends-on): Add getgroups. * lib/group-member.c (group_info, get_group_info): Use gid_t. (group_member): Rely on getgroups replacement. * lib/getugroups.c (getugroups): Use gid_t. * tests/test-getgroups.c (main): Likewise. * NEWS: Mention the signature change. * doc/posix-functions/getgroups.texi (getgroups): Mention the problem with signature. Signed-off-by: Eric Blake <e...@byu.net> --- ChangeLog | 18 ++++++++++++++++++ NEWS | 4 ++++ doc/posix-functions/getgroups.texi | 3 +++ lib/getgroups.c | 34 ++++++++++++++++++++++++++++++++-- lib/getugroups.c | 2 +- lib/getugroups.h | 6 +++--- lib/group-member.c | 29 +++++++++++------------------ lib/unistd.in.h | 2 +- m4/getgroups.m4 | 5 +++-- m4/getugroups.m4 | 8 +++----- modules/group-member | 1 + tests/test-getgroups.c | 2 +- 12 files changed, 81 insertions(+), 33 deletions(-) diff --git a/ChangeLog b/ChangeLog index 968d79d..208c6b1 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,5 +1,23 @@ 2009-11-12 Eric Blake <e...@byu.net> + getgroups: don't expose GETGROUPS_T to user + * lib/getgroups.c (rpl_getgroups): Change signature. Copy array + an element at a time if GETGROUPS_T is wrong size. + * lib/getugroups.h (getugroups): Change signature. + * lib/unistd.in.h (getgroups): Likewise. + * m4/getgroups.m4 (gl_FUNC_GETGROUPS): Use replacement if + signature needs fixing. + * m4/getugroups.m4 (gl_GETUGROUPS): No longer need + AC_TYPE_GETGROUPS. + * modules/group-member (Depends-on): Add getgroups. + * lib/group-member.c (group_info, get_group_info): Use gid_t. + (group_member): Rely on getgroups replacement. + * lib/getugroups.c (getugroups): Use gid_t. + * tests/test-getgroups.c (main): Likewise. + * NEWS: Mention the signature change. + * doc/posix-functions/getgroups.texi (getgroups): Mention the + problem with signature. + getgroups: provide stub for mingw * lib/getgroups.c (getgroups): Provide ENOSYS stub for mingw. * m4/getgroups.m4 (gl_FUNC_GETGROUPS): Check for missing diff --git a/NEWS b/NEWS index a635150..2f364f7 100644 --- a/NEWS +++ b/NEWS @@ -6,6 +6,10 @@ User visible incompatible changes Date Modules Changes +2009-11-12 getgroups These functions now use a signature of gid_t, + getugroups rather than GETGROUPS_T. This probably has no + effect except on very old platforms. + 2009-11-04 tempname The gen_tempname function takes an additional 'suffixlen' argument. You can safely pass 0. diff --git a/doc/posix-functions/getgroups.texi b/doc/posix- functions/getgroups.texi index 951705e..745210e 100644 --- a/doc/posix-functions/getgroups.texi +++ b/doc/posix-functions/getgroups.texi @@ -14,6 +14,9 @@ getgroups @item On Ultrix 4.3, @code{getgroups (0, 0)} always fails. See macro @samp{AC_FUNC_GETGROUPS}. +...@item +On very old systems, this function operated on an array of @samp{int}, +even though that was a different size than an array of @samp{gid_t}. @end itemize Portability problems not fixed by Gnulib: diff --git a/lib/getgroups.c b/lib/getgroups.c index bad676e..e4540fe 100644 --- a/lib/getgroups.c +++ b/lib/getgroups.c @@ -48,14 +48,44 @@ getgroups (int n _UNUSED_PARAMETER_, GETGROUPS_T *groups _UNUSED_PARAMETER_) whether the effective group id is included in the list. */ int -rpl_getgroups (int n, GETGROUPS_T *group) +rpl_getgroups (int n, gid_t *group) { int n_groups; GETGROUPS_T *gbuf; int saved_errno; if (n != 0) - return getgroups (n, group); + { + int result; + int saved_errno; + if (sizeof *group == sizeof *gbuf) + return getgroups (n, (GETGROUPS_T *) group); + + if (n < 0) + { + errno = EINVAL; + return -1; + } + if (SIZE_MAX / sizeof *gbuf <= n) + { + errno = ENOMEM; + return -1; + } + gbuf = malloc (n * sizeof *gbuf); + if (!gbuf) + return -1; + result = getgroups (n, gbuf); + if (0 <= result) + { + n = result; + while (n--) + group[n] = gbuf[n]; + } + saved_errno = errno; + free (gbuf); + errno == saved_errno; + return result; + } n = 20; while (1) diff --git a/lib/getugroups.c b/lib/getugroups.c index 2207b85..b8b4863 100644 --- a/lib/getugroups.c +++ b/lib/getugroups.c @@ -46,7 +46,7 @@ struct group *getgrent (void); Otherwise, return the number of IDs we've written into GROUPLIST. */ int -getugroups (int maxcount, GETGROUPS_T *grouplist, char const *username, +getugroups (int maxcount, gid_t *grouplist, char const *username, gid_t gid) { int count = 0; diff --git a/lib/getugroups.h b/lib/getugroups.h index f2914ad..cbaa664 100644 --- a/lib/getugroups.h +++ b/lib/getugroups.h @@ -1,5 +1,5 @@ /* Get a list of group IDs associated with a specified user ID. - Copyright (C) 2007 Free Software Foundation, Inc. + Copyright (C) 2007, 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 @@ -15,5 +15,5 @@ along with this program. If not, see <http://www.gnu.org/licenses/>. */ #include <sys/types.h> -int getugroups (int maxcount, GETGROUPS_T *grouplist, char const *username, - gid_t gid); +int getugroups (int maxcount, gid_t *grouplist, char const *username, + gid_t gid); diff --git a/lib/group-member.c b/lib/group-member.c index a34efc0..7934c0a 100644 --- a/lib/group-member.c +++ b/lib/group-member.c @@ -1,6 +1,6 @@ /* group-member.c -- determine whether group id is in calling user's group list - Copyright (C) 1994, 1997, 1998, 2003, 2005, 2006 Free Software + Copyright (C) 1994, 1997, 1998, 2003, 2005, 2006, 2009 Free Software Foundation, Inc. This program is free software: you can redistribute it and/or modify @@ -32,11 +32,9 @@ struct group_info { int n_groups; - GETGROUPS_T *group; + gid_t *group; }; -#if HAVE_GETGROUPS - static void free_group_info (struct group_info const *g) { @@ -48,7 +46,7 @@ get_group_info (struct group_info *gi) { int n_groups; int n_group_slots = getgroups (0, NULL); - GETGROUPS_T *group; + gid_t *group; if (n_group_slots < 0) return false; @@ -72,18 +70,14 @@ get_group_info (struct group_info *gi) return true; } -#endif /* not HAVE_GETGROUPS */ - /* Return non-zero if GID is one that we have in our groups list. - If there is no getgroups function, return non-zero if GID matches - either of the current or effective group IDs. */ + Note that the groups list is not guaranteed to contain the current + or effective group ID, so they should generally be checked + separately. */ int group_member (gid_t gid) { -#ifndef HAVE_GETGROUPS - return ((gid == getgid ()) || (gid == getegid ())); -#else int i; int found; struct group_info gi; @@ -96,16 +90,15 @@ group_member (gid_t gid) for (i = 0; i < gi.n_groups; i++) { if (gid == gi.group[i]) - { - found = 1; - break; - } + { + found = 1; + break; + } } free_group_info (&gi); return found; -#endif /* HAVE_GETGROUPS */ } #ifdef TEST @@ -119,7 +112,7 @@ main (int argc, char **argv) program_name = argv[0]; - for (i=1; i<argc; i++) + for (i = 1; i < argc; i++) { gid_t gid; diff --git a/lib/unistd.in.h b/lib/unistd.in.h index 90494e4..c321987 100644 --- a/lib/unistd.in.h +++ b/lib/unistd.in.h @@ -417,7 +417,7 @@ extern int getdtablesize (void); If N is 0, return the group count; otherwise, N describes how many entries are available in GROUPS. Return -1 and set errno if N is not 0 and not large enough. Fails with ENOSYS on some systems. */ -int getgroups (int n, GETGROUPS_T *groups); +int getgroups (int n, gid_t *groups); # endif #elif defined GNULIB_POSIXCHECK # undef getgroups diff --git a/m4/getgroups.m4 b/m4/getgroups.m4 index 1dd39ea..4d96712 100644 --- a/m4/getgroups.m4 +++ b/m4/getgroups.m4 @@ -1,4 +1,4 @@ -# serial 13 +# serial 14 dnl From Jim Meyering. dnl A wrapper around AC_FUNC_GETGROUPS. @@ -17,7 +17,8 @@ AC_DEFUN([gl_FUNC_GETGROUPS], if test "$ac_cv_func_getgroups" != yes; then AC_LIBOBJ([getgroups]) HAVE_GETGROUPS=0 - elif test "$ac_cv_func_getgroups_works" != yes; then + elif test "$ac_cv_func_getgroups_works.$ac_cv_type_getgroups" != yes.gid_t + then AC_LIBOBJ([getgroups]) REPLACE_GETGROUPS=1 fi diff --git a/m4/getugroups.m4 b/m4/getugroups.m4 index 3c5e15e..f34b005 100644 --- a/m4/getugroups.m4 +++ b/m4/getugroups.m4 @@ -1,5 +1,6 @@ -# getugroups.m4 serial 6 -dnl Copyright (C) 2002, 2003, 2005, 2006 Free Software Foundation, Inc. +# getugroups.m4 serial 7 +dnl Copyright (C) 2002, 2003, 2005, 2006, 2009 Free Software +dnl 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. @@ -7,7 +8,4 @@ dnl with or without modifications, as long as this notice is preserved. AC_DEFUN([gl_GETUGROUPS], [ AC_LIBOBJ([getugroups]) - - dnl Prerequisites of lib/getugroups.c. - AC_TYPE_GETGROUPS ]) diff --git a/modules/group-member b/modules/group-member index 45c8bd5..20075dd 100644 --- a/modules/group-member +++ b/modules/group-member @@ -8,6 +8,7 @@ m4/group-member.m4 Depends-on: extensions +getgroups xalloc stdbool diff --git a/tests/test-getgroups.c b/tests/test-getgroups.c index da027d0..8672a90 100644 --- a/tests/test-getgroups.c +++ b/tests/test-getgroups.c @@ -40,7 +40,7 @@ int main (int argc, char **argv _UNUSED_PARAMETER_) { int result; - GETGROUPS_T *groups; + gid_t *groups; errno = 0; result = getgroups (0, NULL); -- 1.6.4.2 >From 5c73d953f8ff5fc8cf1554266fbb05ae46c7e1a9 Mon Sep 17 00:00:00 2001 From: Eric Blake <e...@byu.net> Date: Thu, 12 Nov 2009 11:31:52 -0700 Subject: [PATCH 5/5] mgetgroups: new module, taken from coreutils Wrapper function that makes using getgroups/getugroups easier to use. As part of the move from coreutils, convert GETGROUPS_T to gid_t, and allow mgetgroups(NULL,getegid(),&list) as a way to ensure that the effective gid is in the list. * modules/mgetgroups: New file. * lib/mgetgroups.h: Likewise. * lib/mgetgroups.c (mgetgroups): Likewise. * m4/mgetgroups.m4 (gl_MGETGROUPS): Likewise. * MODULES.html.sh (Users and groups): Mention it. Signed-off-by: Eric Blake <e...@byu.net> --- ChangeLog | 7 +++ MODULES.html.sh | 1 + lib/mgetgroups.c | 149 ++++++++++++++++++++++++++++++++++++++++++++++++++++ lib/mgetgroups.h | 19 +++++++ m4/mgetgroups.m4 | 11 ++++ modules/mgetgroups | 26 +++++++++ 6 files changed, 213 insertions(+), 0 deletions(-) create mode 100644 lib/mgetgroups.c create mode 100644 lib/mgetgroups.h create mode 100644 m4/mgetgroups.m4 create mode 100644 modules/mgetgroups diff --git a/ChangeLog b/ChangeLog index 208c6b1..651588b 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,5 +1,12 @@ 2009-11-12 Eric Blake <e...@byu.net> + mgetgroups: new module, taken from coreutils + * modules/mgetgroups: New file. + * lib/mgetgroups.h: Likewise. + * lib/mgetgroups.c (mgetgroups): Likewise. + * m4/mgetgroups.m4 (gl_MGETGROUPS): Likewise. + * MODULES.html.sh (Users and groups): Mention it. + getgroups: don't expose GETGROUPS_T to user * lib/getgroups.c (rpl_getgroups): Change signature. Copy array an element at a time if GETGROUPS_T is wrong size. diff --git a/MODULES.html.sh b/MODULES.html.sh index 5738ea4..7d42b73 100755 --- a/MODULES.html.sh +++ b/MODULES.html.sh @@ -2580,6 +2580,7 @@ func_all_modules () func_module getugroups func_module group-member func_module idcache + func_module mgetgroups func_module userspec func_end_table diff --git a/lib/mgetgroups.c b/lib/mgetgroups.c new file mode 100644 index 0000000..f68e28f --- /dev/null +++ b/lib/mgetgroups.c @@ -0,0 +1,149 @@ +/* mgetgroups.c -- return a list of the groups a user or current process is in + + Copyright (C) 2007-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/>. */ + +/* Extracted from coreutils' src/id.c. */ + +#include <config.h> + +#include "mgetgroups.h" + +#include <stdlib.h> +#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 gid_t * +realloc_groupbuf (gid_t *g, size_t num) +{ + if (xalloc_oversized (num, sizeof *g)) + { + errno = ENOMEM; + return NULL; + } + + return realloc (g, num * sizeof *g); +} + +/* 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 + group ID (pw_gid) obtained from getpwuid, in case USERNAME is not + listed in the groups database (e.g., /etc/groups). If USERNAME is + NULL, store the supplementary groups of the current process, and GID + should be -1 or the effective group ID (getegid). Upon failure, + don't modify *GROUPS, set errno, and return -1. Otherwise, return + the number of groups. */ + +int +mgetgroups (char const *username, gid_t gid, gid_t **groups) +{ + int max_n_groups; + int ng; + gid_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 + length 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 { N_GROUPS_INIT = 10 }; + max_n_groups = N_GROUPS_INIT; + + g = realloc_groupbuf (NULL, max_n_groups); + if (g == NULL) + return -1; + + while (1) + { + gid_t *h; + int last_n_groups = max_n_groups; + + /* getgrouplist updates max_n_groups to num required. */ + ng = getgrouplist (username, gid, g, &max_n_groups); + + /* Some systems (like Darwin) have a bug where they + never increase max_n_groups. */ + if (ng < 0 && last_n_groups == max_n_groups) + max_n_groups *= 2; + + if ((h = realloc_groupbuf (g, max_n_groups)) == NULL) + { + int saved_errno = errno; + free (g); + errno = saved_errno; + return -1; + } + g = h; + + if (0 <= ng) + { + *groups = g; + /* On success some systems just return 0 from getgrouplist, + so return max_n_groups rather than ng. */ + return max_n_groups; + } + } + } + /* else no username, so fall through and use getgroups. */ +#endif + + max_n_groups = (username + ? getugroups (0, NULL, username, gid) + : getgroups (0, NULL) + (gid != -1)); + + /* If we failed to count groups with NULL for a buffer, + try again with a non-NULL one, just in case. */ + if (max_n_groups < 0) + max_n_groups = 5; + + g = realloc_groupbuf (NULL, max_n_groups); + if (g == NULL) + return -1; + + ng = (username + ? getugroups (max_n_groups, g, username, gid) + : getgroups (max_n_groups, g + (gid != -1))); + + if (ng < 0) + { + int saved_errno = errno; + free (g); + errno = saved_errno; + return -1; + } + + if (!username && gid != -1) + { + *g = gid; + ng++; + } + *groups = g; + return ng; +} diff --git a/lib/mgetgroups.h b/lib/mgetgroups.h new file mode 100644 index 0000000..909d84c --- /dev/null +++ b/lib/mgetgroups.h @@ -0,0 +1,19 @@ +/* Get a list of all group IDs associated with a specified user ID. + Copyright (C) 2007, 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/>. */ + +#include <sys/types.h> + +int mgetgroups (const char *username, gid_t gid, gid_t **groups); diff --git a/m4/mgetgroups.m4 b/m4/mgetgroups.m4 new file mode 100644 index 0000000..3d1958f --- /dev/null +++ b/m4/mgetgroups.m4 @@ -0,0 +1,11 @@ +#serial 4 +dnl Copyright (C) 2007-2009 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_ONCE([getgrouplist]) + AC_LIBOBJ([mgetgroups]) +]) diff --git a/modules/mgetgroups b/modules/mgetgroups new file mode 100644 index 0000000..58ef740 --- /dev/null +++ b/modules/mgetgroups @@ -0,0 +1,26 @@ +Description: +Return the group IDs of a user or current process in malloc'd storage. + +Files: +lib/mgetgroups.c +lib/mgetgroups.h +m4/mgetgroups.m4 + +Depends-on: +getgroups +getugroups +xalloc + +configure.ac: +gl_MGETGROUPS + +Makefile.am: + +Include: +"mgetgroups.h" + +License: +GPL + +Maintainer: +Jim Meyering, Eric Blake -- 1.6.4.2 diff --git i/ChangeLog w/ChangeLog index 6087424..651588b 100644 --- i/ChangeLog +++ w/ChangeLog @@ -4,6 +4,7 @@ * modules/mgetgroups: New file. * lib/mgetgroups.h: Likewise. * lib/mgetgroups.c (mgetgroups): Likewise. + * m4/mgetgroups.m4 (gl_MGETGROUPS): Likewise. * MODULES.html.sh (Users and groups): Mention it. getgroups: don't expose GETGROUPS_T to user diff --git i/lib/mgetgroups.c w/lib/mgetgroups.c index 0ebc2be..f68e28f 100644 --- i/lib/mgetgroups.c +++ w/lib/mgetgroups.c @@ -1,4 +1,4 @@ -/* mgetgroups.c -- return a list of the groups a user is in +/* mgetgroups.c -- return a list of the groups a user or current process is in Copyright (C) 2007-2009 Free Software Foundation, Inc. @@ -29,36 +29,38 @@ #if HAVE_GETGROUPLIST # include <grp.h> #endif + #include "getugroups.h" #include "xalloc.h" - -static GETGROUPS_T * -realloc_groupbuf (GETGROUPS_T *g, size_t num) +static gid_t * +realloc_groupbuf (gid_t *g, size_t num) { - if (xalloc_oversized (num, sizeof (*g))) + if (xalloc_oversized (num, sizeof *g)) { errno = ENOMEM; return NULL; } - return realloc (g, num * sizeof (*g)); + return realloc (g, num * sizeof *g); } /* 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 group ID (pw_gid) obtained from getpwuid, in case USERNAME is not - listed in the groups database (e.g., /etc/groups). Upon failure, + listed in the groups database (e.g., /etc/groups). If USERNAME is + NULL, store the supplementary groups of the current process, and GID + should be -1 or the effective group ID (getegid). Upon failure, don't modify *GROUPS, set errno, and return -1. Otherwise, return the number of groups. */ int -mgetgroups (char const *username, gid_t gid, GETGROUPS_T **groups) +mgetgroups (char const *username, gid_t gid, gid_t **groups) { int max_n_groups; int ng; - GETGROUPS_T *g; + gid_t *g; #if HAVE_GETGROUPLIST /* We prefer to use getgrouplist if available, because it has better @@ -80,7 +82,7 @@ mgetgroups (char const *username, gid_t gid, GETGROUPS_T **groups) while (1) { - GETGROUPS_T *h; + gid_t *h; int last_n_groups = max_n_groups; /* getgrouplist updates max_n_groups to num required. */ @@ -114,7 +116,7 @@ mgetgroups (char const *username, gid_t gid, GETGROUPS_T **groups) max_n_groups = (username ? getugroups (0, NULL, username, gid) - : getgroups (0, NULL)); + : getgroups (0, NULL) + (gid != -1)); /* If we failed to count groups with NULL for a buffer, try again with a non-NULL one, just in case. */ @@ -127,7 +129,7 @@ mgetgroups (char const *username, gid_t gid, GETGROUPS_T **groups) ng = (username ? getugroups (max_n_groups, g, username, gid) - : getgroups (max_n_groups, g)); + : getgroups (max_n_groups, g + (gid != -1))); if (ng < 0) { @@ -137,6 +139,11 @@ mgetgroups (char const *username, gid_t gid, GETGROUPS_T **groups) return -1; } + if (!username && gid != -1) + { + *g = gid; + ng++; + } *groups = g; return ng; } diff --git i/lib/mgetgroups.h w/lib/mgetgroups.h index 4779fec..909d84c 100644 --- i/lib/mgetgroups.h +++ w/lib/mgetgroups.h @@ -16,4 +16,4 @@ #include <sys/types.h> -int mgetgroups (const char *username, gid_t gid, GETGROUPS_T **groups); +int mgetgroups (const char *username, gid_t gid, gid_t **groups); diff --git i/m4/mgetgroups.m4 w/m4/mgetgroups.m4 index 4b3f328..3d1958f 100644 --- i/m4/mgetgroups.m4 +++ w/m4/mgetgroups.m4 @@ -1,4 +1,4 @@ -#serial 3 +#serial 4 dnl Copyright (C) 2007-2009 Free Software Foundation, Inc. dnl This file is free software; the Free Software Foundation dnl gives unlimited permission to copy and/or distribute it, @@ -6,6 +6,6 @@ dnl with or without modifications, as long as this notice is preserved. AC_DEFUN([gl_MGETGROUPS], [ - AC_CHECK_FUNCS([getgrouplist]) + AC_CHECK_FUNCS_ONCE([getgrouplist]) AC_LIBOBJ([mgetgroups]) ])