[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: question about mgetgroups()
From: |
Pádraig Brady |
Subject: |
Re: question about mgetgroups() |
Date: |
Mon, 26 May 2014 22:47:03 +0100 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:17.0) Gecko/20130110 Thunderbird/17.0.2 |
On 05/26/2014 09:00 PM, Denis Excoffier wrote:
> Hello,
>
> In mgetgroups.c, you can read the following piece of code:
>
> --------------------------------------------------
> if (username)
> {
> enum { N_GROUPS_INIT = 10 };
> max_n_groups = N_GROUPS_INIT;
>
> g = realloc_groupbuf (NULL, max_n_groups);
> if (g == NULL)
> return -1;
>
> while (1)
> {
> gid_t *h;
> int last_n_groups = max_n_groups;
>
> /* getgrouplist updates max_n_groups to num required. */
> ng = getgrouplist (username, gid, g, &max_n_groups);
>
> /* Some systems (like Darwin) have a bug where they
> never increase max_n_groups. */
> if (ng < 0 && last_n_groups == max_n_groups)
> max_n_groups *= 2;
>
> if ((h = realloc_groupbuf (g, max_n_groups)) == NULL)
> {
> int saved_errno = errno;
> free (g);
> errno = saved_errno;
> return -1;
> }
> g = h;
>
> if (0 <= ng)
> {
> *groups = g;
> /* On success some systems just return 0 from getgrouplist,
> so return max_n_groups rather than ng. */
> return max_n_groups;
> }
> }
> }
> --------------------------------------------------
>
> Is it really necessary to realloc g when ng >= 0? The answer
> could possibly be "Yes, in order to match with the exact number
> of groups found", in particular in the Darwin case, where for
> exemple 79 groups could remain unused when 81 are needed (here,
> N_GROUPS_INIT=10).
>
> Please observe however that (except in the Darwin case) the
> realloc at the second (and last) iteration does not change the
> allocated size. Therefore, the realloc() + "g = h" assignment
> could possibly be surrounded by
> if (last_n_groups != max_n_groups)
> {
> ...
> }
When I wrote that code in coreutils I was simplifying it,
thus trying to avoid extra conditions, and noticed the fact that glibc will
just ignore the realloc when
passed the same size (see last line of:
http://www.gnu.org/software/libc/manual/html_node/Changing-Block-Size.html)
I.E. I was going for as simple as possible while not necessarily being as
fast as possible on non glibc.
But the question is fair and rather than adding a comment,
I'd add the more general condition you have above.
> or even by
> if (ng < 0 && last_n_groups != max_n_groups)
> {
> ...
> }
> if the answer to the first question is, in fact, "No".
This is done to reduce the allocation when the number of groups is less than 10.
> Or am i missing something? This has some (rather small) impact in
> coreutils/id.
thanks,
Pádraig.