Re: Fwd: Re: [Platform-testers] new snapshot available: coreutils-8.12.178-df9cd

[Jim Meyering]

Markus Duft wrote:

> On 09/07/11 16:13, Jim Meyering wrote:
>> Markus Duft wrote:
>> ...
> [snip]
>>> also, one more thing i saw from the gentoo ebuilds: we're adding those
>>> to CFLAGS for building, as otherwise "id" will be dead slow on
>>> domain-controlled windows machines:
>>>  -Dgetgrgid=getgrgid_nomembers -Dgetgrent=getgrent_nomembers
>>> -Dgetgrnam=getgrnam_nomembers
>>
>> It would help a lot if you would give more detail, if possible,
>> like why the new version is more efficient, and what (if any)
>> is the resulting difference in semantics?
>
> those functions behave exactly like their non-_nomembers counterparts,
> except, that they don't fetch _all_ group members from the domain
> controller. in my case for example, we have > 300 users in the domain,
> which makes each call to the normal versions take > 3 seconds. i'd
> suggest a test for those in configure, and if they exist, use
> them. i'm unsure on how such a test could look like though, my m4-foo
> is not strong enough ;)

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

>>> +#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