Re: mgetgroups improvements

[Eric Blake]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

According to Bruno Haible on 12/9/2009 2:31 AM:
>> +  {
>> +    gid_t first = *g;
> 
> This code will do an invalid memory access if ng == 0 (which can happen if
> gid == (gid_t) -1 and getgroups or getugroups does not find a gid).

Good catch.  And I even thought about it when writing the code; my
justification was that g is memory returned by realloc, and it is
guaranteed to not be NULL (since we use the realloc module for gnu
semantics, it will be at least one byte).  Are there any implementations
of malloc/realloc where malloc(0) does not return dereferencable memory at
least sizeof(gid_t) in length?  Beyond that one access, a length of 0
causes no further problems.

But, theoretical debates aside, I agree that it is be explicitly safe,
rather than accessing uninitialized memory and triggering valgrind
warnings, even if the access will not fault.

> Also, the name of the variable 'sentinel' is confusing, precisely because
> there is no sentinel [1] at the end of the array. It's just a pointer to the
> end of the array.

Bike-shedding, since Pádraig didn't like the name 'last'.  But your
proposal for groups_end is reasonable.

> !   if (ng > 1)

Please use (1 < ng) here, for consistency with the rest of Jim's style.
Other than that, I'm okay with the patch.

- --
Don't work too hard, make some time for fun as well!

Eric Blake             address@hidden
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.9 (Cygwin)
Comment: Public key at home.comcast.net/~ericblake/eblake.gpg
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/

iEYEARECAAYFAksfoJYACgkQ84KuGfSFAYC3fgCfYmt0HO/XZ/+Gb+Cb0DuyyUC4
w2AAoJRAZGk3gdFm/Ht4Nmm6RDar1Ij5
=0jEB
-----END PGP SIGNATURE-----