On 09/08/11 14:00, Jim Meyering wrote: > Markus Duft wrote: >> On 09/08/11 10:17, Jim Meyering wrote: >>> Markus Duft wrote: >>> [snip] > > Thanks. I've pointed out a few issues, hoping you'll adjust > accordingly and resubmit.
sure :) this exercised my git-rebase-foo a lot though ... > >> setgroups is not needed here, as it something different. i have 2 > > I added that conjunct quite deliberately. > If we make the compromise of not detecting the getgr* performance > problems, then we need some other way to ensure that we use the > _nomembers functions only when needed. Appending " && ! HAVE_SETGROUPS" > was my way of saying to use this kludge only on a system that also has > the defect of a missing setgroups function. Yes, that deserves a > comment. hm. ok. i added and commented this, although it feels quite wrong for me. is there any platform that has the _nomembers functions? if yes, would it do any harm to use them? idk... > > ... [snip] > > Please don't add it here, since it is unrelated to this ancient SCO hack. > Add it on a separate line (in jm-macros.m4), with its own comment. heh. while looking for a location to add the check, i realized there is one already! :) so i ommitted this step. hope thats ok, my build goes through, but for obvious reasons this is not significant (as i'll never have it defined). a quick grep over the build tree yields this: mduft coreutils-8.12.193-d8dc8 $ find . -type f | xargs grep SETGROUPS ./autom4te.cache/traces.0:m4trace:configure.ac:46: -1- AH_OUTPUT([HAVE_SETGROUPS], [/* Define to 1 if you have the `setgroups\' function. */ ./autom4te.cache/traces.0:@%:@undef HAVE_SETGROUPS]) ./ChangeLog-2006: * src/setuidgid.c (main) [! HAVE_SETGROUPS]: Don't call setgroups. ./lib/config.h:/* #undef HAVE_SETGROUPS */ ./lib/config.hin:#undef HAVE_SETGROUPS ./lib/config.hin~:#undef HAVE_SETGROUPS ./src/chroot.c:#if ! HAVE_SETGROUPS ./src/setuidgid.c:#if HAVE_SETGROUPS ./src/system.h: other than interix, check for HAVE_SETGROUPS, as interix is ./src/system.h:#if ! HAVE_SETGROUPS > [snip] hope the new versions are acceptable. otherwise, i'll be happy to adjust once more, and resubmit tomorrow :) regards, markus
>From 87c5bf012268222808aee3a9d9ce5729be46828a Mon Sep 17 00:00:00 2001 From: Markus Duft <[email protected]> Date: Thu, 8 Sep 2011 11:18:17 +0200 Subject: [PATCH 1/2] build: accommodate missing setgroups on Interix this adds the check plus a dummy, non-functional, always-successful replacement function, to keep the original code untouched and simple. * src/chroot.c (setgroups) [! HAVE_SETGROUPS]: Define. --- src/chroot.c | 13 +++++++++++++ 1 files changed, 13 insertions(+), 0 deletions(-) diff --git a/src/chroot.c b/src/chroot.c index 95c227b..4c56bf3 100644 --- a/src/chroot.c +++ b/src/chroot.c @@ -52,6 +52,19 @@ static struct option const long_opts[] = {NULL, 0, NULL, 0} }; +#if ! HAVE_SETGROUPS +/* At least Interix lacks supplemental group support. Define an + always-successful replacement to avoid checking for setgroups + availability everywhere, just to support broken platforms. */ +static int +setgroups(size_t size, gid_t const *list) +{ + (void)size; + (void)list; + return 0; +} +#endif + /* Call setgroups to set the supplementary groups to those listed in GROUPS. GROUPS is a comma separated list of supplementary groups (names or numbers). Parse that list, converting any names to numbers, and call setgroups on the -- 1.7.6.1
>From 0687e0aea8156b368e1316d68d0c0bac791ad147 Mon Sep 17 00:00:00 2001 From: Markus Duft <[email protected]> Date: Thu, 8 Sep 2011 13:09:27 +0200 Subject: [PATCH 2/2] build: use getgr*_nomembers functions on interix. interix provides faster replacements for getgr{gid,nam,ent} where group member information is not fetched from domain controllers. this makes 'id' usable on domain controlled interix boxes. * m4/jm-macros.m4: check for _nomembers functions. * src/system.h: redefine function to _nomembers when available. --- m4/jm-macros.m4 | 9 +++++++++ src/system.h | 18 ++++++++++++++++++ 2 files changed, 27 insertions(+), 0 deletions(-) diff --git a/m4/jm-macros.m4 b/m4/jm-macros.m4 index 58b000d..a6de8f1 100644 --- a/m4/jm-macros.m4 +++ b/m4/jm-macros.m4 @@ -89,6 +89,15 @@ AC_DEFUN([coreutils_MACROS], tcgetpgrp \ ) + # those checks exists for interix, where there are blazingly fast + # replacements for some functions, that don't query the domain + # controller for user information where it is not needed. + AC_CHECK_FUNCS_ONCE( \ + getgrgid_nomembers \ + getgrnam_nomembers \ + getgrent_nomembers \ + ) + dnl This can't use AC_REQUIRE; I'm not quite sure why. cu_PREREQ_STAT_PROG diff --git a/src/system.h b/src/system.h index 107dbd5..d8f412c 100644 --- a/src/system.h +++ b/src/system.h @@ -213,6 +213,24 @@ struct passwd *getpwuid (); struct group *getgrgid (); #endif +/* Interix has replacements for getgr{gid,nam,ent}, that don't + query the domain controller for group members when not required. + This speeds up the calls tremendously (<1 ms vs. >3 s). */ +/* To protect any system that could provide _nomembers functions + other than interix, check for HAVE_SETGROUPS, as interix is + one of the very few (the only?) platform that lacks it */ +#if ! HAVE_SETGROUPS +# if HAVE_GETGRGID_NOMEMBERS +# define getgrgid(gid) getgrgid_nomembers(gid) +# endif +# if HAVE_GETGRNAM_NOMEMBERS +# define getgrnam(nam) getgrnam_nomembers(nam) +# endif +# if HAVE_GETGRENT_NOMEMBERS +# define getgrent() getgrent_nomembers() +# endif +#endif + #if !HAVE_DECL_GETUID uid_t getuid (); #endif -- 1.7.6.1
