On 09/08/11 10:17, Jim Meyering wrote:
> Markus Duft wrote:
>
[snip]
>
> A simpler approach might be ok: add a test for existence of the
> _nomembers functions (in m4/jm-macros.m4), and then add code like
> this for each in system.h, inserted after the declaration of getgrgid:
>
> /* Add a comment here, justifying this, based on what you
> said above. */
> #if HAVE_GETGRGID_NOMEMBERS && ! HAVE_SETGROUPS
> # define getgrgid(n) getgrgid_nomembers(n)
> #endif
>
> #if HAVE_GETGRNAM_NOMEMBERS && ! HAVE_SETGROUPS
> ...
>
> Please include a complete patch that I can apply with
> "git am FILE", per instructions in HACKING:
>
> http://git.sv.gnu.org/cgit/coreutils.git/tree/HACKING
ok. setgroups is not needed here, as it something different. i have 2 commits
on top of current master now (attached).
i'll investigate on the long double problem some more. currently i think, that
it could be the case, that libc has been built with a compiler with 64bit long
doubles, whereas the current gcc has 128 bit long double (not verified yet
though). i'll have a look... :)
regards,
markus
>
>>>> +#ifdef __INTERIX
>>>> +// very very broken long double "support".
>>>
>>> Thanks for the suggested patch, but what problem are you working around?
>>> Is this due to insufficient compiler support? Use a newer gcc?
>>> Insufficient library support? If that's it, then won't one of the
>>> gnulib's replacement functions work for you?
>>
>> gcc is 4.2.4 and there is no chance to get a newer one to work
>> currently (i have big problems forward porting my patches). but i
>> /think/ it's the library support anyway thats broken. it seems to be
>> somewhere in the printing (fprintf, sprintf). if i run an unpatched
>> "seq 1 10", i get this:
>>
>> mduft coreutils-8.12.193-d8dc8 $ ./src/seq 1 10
>> 0
>> 0
>> -2
>> 0
>> -0
>> -2
>> -26815615859885194199148049996411692254958731641184786755447122887443528060147093953603748596333806855380063716372972101707507765623893139892867298012168192
>> 0
>> -0
>> -0
>>
>> in [1], Bruno Haible suggested some test from the gnulib test-suite,
>> with which i can investigate the problem more deeply, which i'm now
>> doing (as time allows).
>
> Please pursue that first.
> If improving the printing or conversion functions in gnulib
> can solve your problem, then making changes to coreutils would
> be counterproductive.
>
>> diff -ru -x '*.o' coreutils-8.12.193-d8dc8.orig/src/chroot.c
>> coreutils-8.12.193-d8dc8/src/chroot.c
>
> I suggested to use #if ! HAVE_SETGROUPS, yet you used #ifdef __INTERIX.
> Why?
>
>> +#ifdef __INTERIX
>
> Using "#ifdef ANYTHING_HARDWARE_SPECIFIC" is best avoided.
> We prefer so-called feature tests like "#if ! HAVE_SETGROUPS",
> because then when some other system with the same problem comes
> along, rather than having to adjust every #ifdef like this:
>
> #if defined __INTERIX || defined __other_broken_system
>
> our "#if ! HAVE_SETGROUPS" just works.
> That is much more scalable from the software engineering/maintenance
> point of view.
>
>> +/* Interix lacks supplemental group support. A dummy, always successful
>> + * replacement is added here to avoid the need to check for setgroups,
>> + * just to support such a broken platform. */
>> +static int setgroups(size_t size, gid_t const *list)
>> +{
>> + (void)size; (void)list;
>> + return 0;
>> +}
>> +#endif
>From eedb653448463aff1db947c4e94d63c7609a3c4f 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] add check for setgroups(), which may be missing on some
platforms.
this adds the check plus a dummy, non-functional, always-successful
replacement function, to keep the original code untouched and simple.
Signed-off-by: Markus Duft <[email protected]>
---
configure.ac | 2 +-
src/chroot.c | 11 +++++++++++
2 files changed, 12 insertions(+), 1 deletions(-)
diff --git a/configure.ac b/configure.ac
index 291b19e..c22f409 100644
--- a/configure.ac
+++ b/configure.ac
@@ -199,7 +199,7 @@ if test $utils_cv_localtime_cache = yes; then
fi
# SCO-ODT-3.0 is reported to need -los to link programs using initgroups
-AC_CHECK_FUNCS([initgroups])
+AC_CHECK_FUNCS([initgroups setgroups])
if test $ac_cv_func_initgroups = no; then
AC_CHECK_LIB([os], [initgroups])
fi
diff --git a/src/chroot.c b/src/chroot.c
index 95c227b..a9b4b87 100644
--- a/src/chroot.c
+++ b/src/chroot.c
@@ -52,6 +52,17 @@ static struct option const long_opts[] =
{NULL, 0, NULL, 0}
};
+#ifndef HAVE_SETGROUPS
+/* Some systems (like interix) lack supplemental group support. A dummy,
+ * always successful replacement is added here to avoid the need to check
+ * 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 1ef299a66faa06becdd9bdb30ee6736a823e184b 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] add support for 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.
Signed-off-by: Markus Duft <[email protected]>
---
m4/jm-macros.m4 | 9 +++++++++
src/system.h | 13 +++++++++++++
2 files changed, 22 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..3e44f89 100644
--- a/src/system.h
+++ b/src/system.h
@@ -213,6 +213,19 @@ 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), so use them. */
+#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
+
#if !HAVE_DECL_GETUID
uid_t getuid ();
#endif
--
1.7.6.1
