From patchwork Thu Mar 12 23:01:11 2015 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Paul Eggert X-Patchwork-Id: 5600 Received: (qmail 15696 invoked by alias); 12 Mar 2015 23:01:16 -0000 Mailing-List: contact libc-alpha-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: libc-alpha-owner@sourceware.org Delivered-To: mailing list libc-alpha@sourceware.org Received: (qmail 15686 invoked by uid 89); 12 Mar 2015 23:01:15 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-3.2 required=5.0 tests=AWL, BAYES_00, SPF_PASS, T_RP_MATCHES_RCVD autolearn=ham version=3.3.2 X-HELO: smtp.cs.ucla.edu Message-ID: <55021AB7.1060905@cs.ucla.edu> Date: Thu, 12 Mar 2015 16:01:11 -0700 From: Paul Eggert User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.5.0 MIME-Version: 1.0 To: Roland McGrath CC: Paul Pluzhnikov , Szabolcs Nagy , Joseph Myers , GLIBC Devel , "mtk@man7.org" Subject: Re: [patch] Error on setenv(..., NULL, ...) References: <55008721.1090200@arm.com> <55008DE0.8050805@cs.ucla.edu> <20150312215314.1B7CC2C3B8E@topped-with-meat.com> In-Reply-To: <20150312215314.1B7CC2C3B8E@topped-with-meat.com> 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. From 1441ea7a5cfb78e36c8e6f170a30de52fd01920e Mon Sep 17 00:00:00 2001 From: Paul Eggert Date: Thu, 12 Mar 2015 15:57:07 -0700 Subject: [PATCH] * stdlib/setenv.c (__add_to_environ): Dump core quickly if setenv (..., NULL, ...) is called. --- ChangeLog | 5 +++++ stdlib/setenv.c | 10 +++++++++- 2 files changed, 14 insertions(+), 1 deletion(-) diff --git a/ChangeLog b/ChangeLog index d6526e9..56b3d73 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,8 @@ +2015-03-12 Paul Eggert + + * stdlib/setenv.c (__add_to_environ): + Dump core quickly if setenv (..., NULL, ...) is called. + 2015-03-12 Joseph Myers * soft-fp/soft-fp.h (_FP_STATIC_ASSERT): New macro. diff --git a/stdlib/setenv.c b/stdlib/setenv.c index b60c4f0..0534236 100644 --- a/stdlib/setenv.c +++ b/stdlib/setenv.c @@ -114,8 +114,16 @@ __add_to_environ (name, value, combined, replace) { char **ep; size_t size; + + /* Compute lengths before locking, so that the critical section is + less of a performance bottleneck. VALLEN is needed only if + COMBINED is non-null. Also, testing COMBINED instead of VALUE + causes setenv (..., NULL, ...) to dump core now instead of + corrupting memory later. */ const size_t namelen = strlen (name); - const size_t vallen = value != NULL ? strlen (value) + 1 : 0; + size_t vallen; + if (combined != NULL) + vallen = strlen (value) + 1; LOCK; -- 2.1.0