Re: inifite loop in recent mgetgroups.c:mgetgroups on Darwin/MacOS

[Jim Meyering]

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.