[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: inifite loop in recent mgetgroups.c:mgetgroups on Darwin/MacOS
From: |
Jim Meyering |
Subject: |
Re: inifite loop in recent mgetgroups.c:mgetgroups on Darwin/MacOS |
Date: |
Thu, 09 Apr 2009 14:55:03 +0200 |
Pádraig Brady wrote:
> Jim Meyering wrote:
>> Pádraig Brady wrote:
>> ..
>>
>> Hi Pádraig,
>> Thanks for working on that.
>>
>> This NEWS entry makes it look like these are coreutils bugs.
>> True, it's a regression, but not due to a bug in coreutils,
>> unless you count the decision to use POSIX getgroups as the bug ;-)
>> The old, inefficient version used to work well enough on those systems,
>> but the new, improved-to-use-POSIX-getgroups version failed due
>> to non-conforming getgroups on Darwin and NetBSD.
>
> I looked for getgrouplist in the POSIX spec but couldn't find it.
Argh. I'm glad you're paying attention. getgrouplist, not getgroups.
I went to look up the former, and looked up getgroups instead.
> There was some talk about adding it to POSIX but I don't know if that went
> anywhere?
> https://www.opengroup.org/sophocles/show_mail.tpl?source=L&listname=austin-review-l&id=2576
>
>>> @@ -97,7 +103,9 @@ mgetgroups (char const *username, gid_t gid, GETGROUPS_T
>>> **groups)
>>> if (0 <= ng)
>>> {
>>> *groups = g;
>>> - return ng;
>>> + /* Some systems just return 0 or -1 from getgrouplist,
>>> + so return max_n_groups rather than ng. */
>>> + return max_n_groups;
>>
>> If it returns -1, then the above isn't reached.
>> Would this work as well?
>>
>> /* Some vendor getgrouplist functions return 0,
>> so return max_n_groups rather than 0. */
>> return ng ? ng : max_n_groups;
>
> It would. The advantage of doing that is it would handle the very unlikely
> case of getgrouplist implementations that don't update max_n_groups at all.
> The disadvantage is that it would add an extra conditional and depend on
> getgrouplist not returning 1 on success which is more likely I think.
>
> Updated patch attached.
Ah. I see the point.
However, please remove the " or -1" mention in the comment,
since only the ng == 0 case is relevant in that block.
Thanks.
- inifite loop in recent mgetgroups.c:mgetgroups on Darwin/MacOS, Steven Parkes, 2009/04/07
- RE: inifite loop in recent mgetgroups.c:mgetgroups on Darwin/MacOS, Steven Parkes, 2009/04/07
- Re: inifite loop in recent mgetgroups.c:mgetgroups on Darwin/MacOS, Pádraig Brady, 2009/04/08
- RE: inifite loop in recent mgetgroups.c:mgetgroups on Darwin/MacOS, Steven Parkes, 2009/04/08
- Re: inifite loop in recent mgetgroups.c:mgetgroups on Darwin/MacOS, Pádraig Brady, 2009/04/08
- RE: inifite loop in recent mgetgroups.c:mgetgroups on Darwin/MacOS, Steven Parkes, 2009/04/08
- Re: inifite loop in recent mgetgroups.c:mgetgroups on Darwin/MacOS, Jim Meyering, 2009/04/09
- Re: inifite loop in recent mgetgroups.c:mgetgroups on Darwin/MacOS, Pádraig Brady, 2009/04/09
- Re: inifite loop in recent mgetgroups.c:mgetgroups on Darwin/MacOS,
Jim Meyering <=