Re: [patch] Error on setenv(..., NULL, ...)

[Paul Eggert <eggert at cs dot ucla dot edu>]

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;