question about mgetgroups()

[Denis Excoffier]

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)
      {
        ...
      }
or even by
    if (ng < 0 && last_n_groups != max_n_groups)
      {
        ...
      }
if the answer to the first question is, in fact, "No".


Or am i missing something? This has some (rather small) impact in coreutils/id.

Best regards,

Denis Excoffier.