bug-gnulib
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: getgroups improvements


From: Jim Meyering
Subject: Re: getgroups improvements
Date: Fri, 13 Nov 2009 10:14:43 +0100

Eric Blake wrote:
> 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

Good catch on that bug fix.
And hiding GETGROUPS_T is definitely the way to go.
These changes look fine.  I'll test via coreutils, after you push.
I guess no one who used the replacement and who had users with 20 or
more groups ever noticed.

Thanks!




reply via email to

[Prev in Thread] Current Thread [Next in Thread]