[PATCH] cyrus-imapd: use getgrouplist instead of set/getgrent on systems supporting it

Tomas Janousek tjanouse at redhat.com
Mon Sep 17 10:07:48 EDT 2007


Hello,

On Mon, Sep 17, 2007 at 09:49:38AM -0400, Ken Murchison wrote:
> I applied Simon's version to CVS, but then I realized that it causes 
> myfreestate() to crash on my Linux box, because the newstate->groups array 
> doesn't get entirely populated.  The following patch fixes the problem of 

Oh, sorry for that.

> having gaps in newstate->groups, but I'm not sure why we're skipping groups 
> anyways.  Also, will getgrouplist() really return -1 for anything other than 
> the output array being too small (wondering if we need the 'do' loop)?

Yes, it returns -1 also in case of an error. It's not in the manpage but I
looked at the glibc sources (at the time when I wrote the patch, so I may be
inaccurate).

> --- auth_unix.c.~1.42.~	2007-09-13 13:24:42.000000000 -0400
> +++ auth_unix.c	2007-09-17 09:38:45.000000000 -0400
> @@ -270,11 +270,14 @@
>  	goto err;
>      }
>
> -    newstate->group = (char **)xmalloc(newstate->ngroups * sizeof(char *));
> -    for (i = 0; i < newstate->ngroups; ++i ) {
> +    newstate->ngroups = 0;
> +    newstate->group = (char **)xmalloc(ngroups * sizeof(char *));

ngroups needn't be the same as newstate->ngroups, I think.

> +    for (i = 0; i < ngroups; i++) {
>  	if (pwd || groupids[i] != gid) {
> -	    if ((grp = getgrgid(groupids[i])))
> -		newstate->group[i] = xstrdup(grp->gr_name);
> +	    if ((grp = getgrgid(groupids[i]))) {
> +		newstate->ngroups++;
> +		newstate->group[newstate->ngroups-1] = xstrdup(grp->gr_name);
> +	    }
>  	}
>      }

Regards,
-- 
Tomas Janousek, SW Engineer, Red Hat, Inc.


More information about the Cyrus-devel mailing list