Date: Thu, 6 Jul 2006 07:37:00 -0400 From: John Baldwin <jhb@freebsd.org> To: Scott Long <scottl@freebsd.org> Cc: Perforce Change Reviews <perforce@freebsd.org> Subject: Re: PERFORCE change 100686 for review Message-ID: <200607060737.01194.jhb@freebsd.org> In-Reply-To: <200607060348.k663mTHW007992@repoman.freebsd.org> References: <200607060348.k663mTHW007992@repoman.freebsd.org>
next in thread | previous in thread | raw e-mail | index | archive | help
On Wednesday 05 July 2006 23:48, Scott Long wrote: > http://perforce.freebsd.org/chv.cgi?CH=100686 > > Change 100686 by scottl@scottl-wv1u on 2006/07/06 03:47:59 > > Use a sleep mutex to protect kernel environment handling instead of > an sx lock. The sx lock seemed to only be used to get around the > copyout case in kenv(KENV_DUMP) path. Fix that path to safely use a > sleep lock instead. > > Affected files ... > > .. //depot/projects/scottl-camlock/src/sys/kern/kern_environment.c#7 edit > .. //depot/projects/scottl-camlock/src/sys/kern/subr_hints.c#3 edit > .. //depot/projects/scottl-camlock/src/sys/sys/systm.h#7 edit > > Differences ... > > ==== //depot/projects/scottl-camlock/src/sys/kern/kern_environment.c#7 (text+ko) ==== > > @@ -100,7 +99,8 @@ > return (error); > #endif > done = needed = 0; > - sx_slock(&kenv_lock); > + buffer = malloc(uap->len, M_TEMP, M_WAITOK|M_ZERO); > + mtx_lock(&kenv_lock); > for (i = 0; kenvp[i] != NULL; i++) { > len = strlen(kenvp[i]) + 1; > needed += len; > @@ -110,14 +110,15 @@ > * buffer, just keep computing the required size. > */ > if (uap->value != NULL && len > 0) { > - error = copyout(kenvp[i], uap->value + done, > - len); > - if (error) > + if (done + len > uap->len) > break; > + bcopy(kenvp[i], buffer + done, len); > done += len; > } > } > - sx_sunlock(&kenv_lock); > + mtx_unlock(&kenv_lock); > + error = copyout(buffer, uap->value, done); > + free(buffer, M_TEMP); > td->td_retval[0] = ((done == needed) ? 0 : needed); > return (error); > } Minor nit here is that you probably can avoid the malloc() and skip the copyout() and free() if uap->value == NULL. In that case the caller is just asking for the length to know how big of a buffer to malloc. -- John Baldwin
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?200607060737.01194.jhb>