Hi,

>>>>> Sat, 27 Oct 2007 02:31:32 +0900,
>>>>> Hajimu UMEMOTO <[EMAIL PROTECTED]> said:

ume> The code is suspicious to me.  Isn't the test of `ret != -1' is
ume> opposite?
ume> Further, it seems that the test of `ngroups == newstate->ngroups'
ume> assumes that newstate->ngroups holds the actual number of groups
ume> found, by calling getgrouplist() in the first place.
ume> Perhaps, it should be:

ume>     do {
ume>    groupids = (gid_t *)xrealloc((gid_t *)groupids,
ume>                                 ngroups * sizeof(gid_t));

ume>    newstate->ngroups = ngroups; /* copy of ngroups for comparision */
ume>    ret = getgrouplist(identifier, gid, groupids, &ngroups);
ume>    /*
ume>     * This is tricky. We do this as long as getgrouplist tells us to
ume>     * realloc _and_ the number of groups changes. It tells us to realloc
ume>     * also in the case of failure...
ume>     */
ume>     } while (ret == -1 && ngroups == newstate->ngroups);
ume>              ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

ume> I'm not sure that we can expect `ngroups == newstate->ngroups',
ume> though.

Oops, I forgot to increase ngroups for next attempt.

Sincerely,

--
Hajimu UMEMOTO @ Internet Mutual Aid Society Yokohama, Japan
[EMAIL PROTECTED]  [EMAIL PROTECTED],jp.}FreeBSD.org
http://www.imasy.org/~ume/
----
Cyrus Home Page: http://cyrusimap.web.cmu.edu/
Cyrus Wiki/FAQ: http://cyrusimap.web.cmu.edu/twiki
List Archives/Info: http://asg.web.cmu.edu/cyrus/mailing-list.html

Reply via email to