This is the mail archive of the
libc-alpha@sourceware.org
mailing list for the glibc project.
Re: [patch] Error on setenv(..., NULL, ...)
- From: Paul Eggert <eggert at cs dot ucla dot edu>
- To: 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: Thu, 12 Mar 2015 16:01:11 -0700
- 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>
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 <eggert@cs.ucla.edu>
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 <eggert@cs.ucla.edu>
+
+ * stdlib/setenv.c (__add_to_environ):
+ Dump core quickly if setenv (..., NULL, ...) is called.
+
2015-03-12 Joseph Myers <joseph@codesourcery.com>
* 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
- 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, ...)