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: Joseph Myers <joseph at codesourcery dot com>
- Cc: Paul Pluzhnikov <ppluzhnikov at google dot com>, Roland McGrath <roland at hack dot frob dot com>, Szabolcs Nagy <szabolcs dot nagy at arm dot com>, GLIBC Devel <libc-alpha at sourceware dot org>, "mtk at man7 dot org" <mtk at man7 dot org>
- Date: Mon, 16 Mar 2015 17:03:05 -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> <55021AB7 dot 1060905 at cs dot ucla dot edu> <20150313170436 dot BF4C92C3B3B at topped-with-meat dot com> <55031BC3 dot 6070709 at cs dot ucla dot edu> <CALoOobMF_eRjg93DDfkTtcyrvCrHBYX8w3CaPU-R62cpOqbiMg at mail dot gmail dot com> <CALoOobO79BAzDZuq3=KJ=DnhVeu7np=W5kthdZrHEG5U1f2k4A at mail dot gmail dot com> <CALoOobNJ2wGtAn=LRwfLFR7o8idxg4+Lgz=jWo08Dxdj_BOHvA at mail dot gmail dot com> <55062712 dot 4040104 at cs dot ucla dot edu> <CALoOobOOGvTPFmdGVGMpdg3eUo_ipLWztOXfgTtAeX93PEK=fg at mail dot gmail dot com> <55065804 dot 2040402 at cs dot ucla dot edu> <alpine dot DEB dot 2 dot 10 dot 1503162347090 dot 30120 at digraph dot polyomino dot org dot uk>
Joseph Myers wrote:
Are you saying that if you include
<libc-internal.h>, then use the macros, then include other headers, that
it doesn't work?
Yes it doesn't work, because libc-internal.h includes string.h before the
including file can invoke the macros. For example, the attached patch does not
work, with the symptoms given below. (Another issue is that the file is
supposed to be sharable with Gnulib, which can't assume libc-internal.h.)
In file included from ../include/bits/string2.h:1:0,
from ../string/string.h:630,
from ../include/string.h:51,
from ../sysdeps/generic/hp-timing-common.h:40,
from ../sysdeps/x86_64/hp-timing.h:38,
from ../include/libc-internal.h:7,
from setenv.c:23:
setenv.c: In function â__add_to_environâ:
../string/bits/string2.h:206:37: error: âvallenâ may be used uninitialized in
this function [-Werror=maybe-uninitialized]
# define __mempcpy(dest, src, n) __builtin_mempcpy (dest, src, n)
^
setenv.c:135:10: note: âvallenâ was declared here
size_t vallen;
diff --git a/stdlib/setenv.c b/stdlib/setenv.c
index b60c4f0..54fc0be 100644
--- a/stdlib/setenv.c
+++ b/stdlib/setenv.c
@@ -19,6 +19,16 @@
# include <config.h>
#endif
+#if _LIBC
+# include <libc-internal.h>
+
+/* Pacify GCC; see the commentary about VALLEN below. This is needed
+ at least through GCC 4.9.2. Pacify GCC for the entire file, as
+ there seems to be no way to pacify GCC selectively, only for the
+ place where it's needed. */
+DIAG_IGNORE_NEEDS_COMMENT (4.9, "-Wmaybe-uninitialized")
+#endif
+
#include <errno.h>
#if !_LIBC
# if !defined errno && !defined HAVE_ERRNO_DECL
@@ -114,8 +124,17 @@ __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 null (unfortunately GCC is not smart enough to deduce
+ this; see the #pragma at the start of this file). 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;
- 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, ...)
- 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, ...)
- Re: [patch] Error on setenv(..., NULL, ...)
- Re: [patch] Error on setenv(..., NULL, ...)