"Ryan Thomas" <[EMAIL PROTECTED]> writes: > I've recently run into an issue with autofs 5 when using groups with > a large number of members. The issue is that getgrgid_r doesn't > always fit in a _SC_GETGR_R_SIZE_MAX sized buffer. Here is a patch > that solves this problem by attempting larger and larger buffer > sizes if the group structure exceeds _SC_GETGR_R_SIZE_MAX.
Kudos for diving in and fixing it! As Ian mentions in the bugzilla you filed, it's surprising that _SC_GETGR_R_SIZE_MAX is not actually accurate! It's probably worth bringing up to the libc folks to see if that's expected. Anyway, comments below. -Jeff > This patch is very similar to the one that recently went into the NFS4 > libnfsidmap code. See http://marc.info/?l=linux-nfsv4&m=116311577732733&w=2 > > > --- autofs-5.0.1/daemon/indirect.c.biggroup 2007-08-29 07:26:25.000000000 > -0400 > +++ autofs-5.0.1/daemon/indirect.c 2007-08-29 07:27:11.000000000 -0400 > @@ -844,16 +844,27 @@ > goto cont; > } > > - gr_tmp = malloc(tmplen + 1); > - if (!gr_tmp) { > - error(ap->logopt, "failed to malloc buffer for getgrgid_r"); > - free(tsv->user); > - free(tsv->home); > - free(tsv); > - goto cont; > - } > + do { > + status = ENOMEM; > + > + gr_tmp = malloc(tmplen + 1); Why not use realloc? That will get rid of all of the freeing business on every iteration. > + if (!gr_tmp) { > + error(ap->logopt, "failed to malloc buffer for getgrgid_r"); > + free(tsv->user); > + free(tsv->home); > + free(tsv); > + goto cont; > + } > + pgr = &gr; > + status = getgrgid_r(mt->gid, pgr, gr_tmp, tmplen, ppgr); > + if (ppgr == NULL && !status) > + status = ENOENT; > + if (status == ERANGE) { > + tmplen *= 2; I don't think you want to double every time. That could get really big, really fast! > + free(gr_tmp); > + } > + } while (status == ERANGE); > > - status = getgrgid_r(mt->gid, pgr, gr_tmp, tmplen, ppgr); > if (status) { > error(ap->logopt, "failed to get group info from getgrgid_r"); > free(tsv->user); > --- autofs-5.0.1/daemon/direct.c.biggroup 2007-08-29 07:26:25.000000000 > -0400 > +++ autofs-5.0.1/daemon/direct.c 2007-08-29 07:27:01.000000000 -0400 > @@ -1364,16 +1364,27 @@ > goto cont; > } > > - gr_tmp = malloc(tmplen + 1); > - if (!gr_tmp) { > - error(ap->logopt, "failed to malloc buffer for getgrgid_r"); > - free(tsv->user); > - free(tsv->home); > - free(tsv); > - goto cont; > - } > + do { > + status = ENOMEM; > + > + gr_tmp = malloc(tmplen + 1); > + if (!gr_tmp) { > + error(ap->logopt, "failed to malloc buffer for getgrgid_r"); > + free(tsv->user); > + free(tsv->home); > + free(tsv); > + goto cont; > + } > + pgr = &gr; > + status = getgrgid_r(mt->gid, pgr, gr_tmp, tmplen, ppgr); > + if (ppgr == NULL && !status) > + status = ENOENT; > + if (status == ERANGE) { > + tmplen *= 2; > + free(gr_tmp); > + } > + } while (status == ERANGE); > > - status = getgrgid_r(mt->gid, pgr, gr_tmp, tmplen, ppgr); > if (status) { > error(ap->logopt, "failed to get group info from getgrgid_r"); > free(tsv->user); And here we see the same code duplicated? Shame on you, Ian! ;) -Jeff _______________________________________________ autofs mailing list [email protected] http://linux.kernel.org/mailman/listinfo/autofs
