This is the mail archive of the
libc-alpha@sourceware.org
mailing list for the glibc project.
Re: [patch] Error on setenv(..., NULL, ...)
- From: "Carlos O'Donell" <carlos at redhat dot com>
- To: Paul Eggert <eggert at cs dot ucla dot edu>, Roland McGrath <roland at hack dot frob dot com>
- Cc: Paul Pluzhnikov <ppluzhnikov at google dot com>, Szabolcs Nagy <szabolcs dot nagy at arm dot com>, Joseph Myers <joseph at codesourcery dot com>, GLIBC Devel <libc-alpha at sourceware dot org>, "mtk at man7 dot org" <mtk at man7 dot org>
- Date: Fri, 13 Mar 2015 10:08:53 -0400
- Subject: Re: [patch] Error on setenv(..., NULL, ...)
- Authentication-results: sourceware.org; auth=none
- References: <CALoOobNSbWUkd_i-L6U0ovbqPYnJY-h=ftX1K61yb19pmJj6aw at mail dot gmail dot com> <alpine dot DEB dot 2 dot 10 dot 1503111712240 dot 30954 at digraph dot polyomino dot org dot uk> <CALoOobPKxfJfnbcUKH8osgCZMeSiD83K1OiF+_vSeAy0ewe1Jw at mail dot gmail dot com> <55008721 dot 1090200 at arm dot com> <CALoOobNbOgm5=oFbEUmTbca3M-KqSUgGmTeWYOt1zTN-CTLoog at mail dot gmail dot com> <55008DE0 dot 8050805 at cs dot ucla dot edu> <20150312215314 dot 1B7CC2C3B8E at topped-with-meat dot com> <55021AB7 dot 1060905 at cs dot ucla dot edu>
On 03/12/2015 07:01 PM, Paul Eggert wrote:
> On 03/12/2015 02:53 PM, Roland McGrath wrote:
>> The only reason I can see not
>> to do that is to force the fault to be before the lock is taken. If that
>> is the explicit intent of the code, then its comments should say so.
>
> I had assumed that lengths are computed before locking to minimize
> the size of the critical section, making it less of a bottleneck.
> Attached is a revised (untested) patch with comments saying that.
Agreed.
This looks good to me. I like the fail early, and it follows the
project's desire to make that visible to users and a debugger at
the point you make the call.
Cheers,
Carlos.
- References:
- [patch] Error on setenv(..., NULL, ...)
- Re: [patch] Error on setenv(..., NULL, ...)
- Re: [patch] Error on setenv(..., NULL, ...)
- Re: [patch] Error on setenv(..., NULL, ...)
- Re: [patch] Error on setenv(..., NULL, ...)
- Re: [patch] Error on setenv(..., NULL, ...)
- Re: [patch] Error on setenv(..., NULL, ...)
- Re: [patch] Error on setenv(..., NULL, ...)