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

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

Paul Pluzhnikov wrote:
> So it looks to me like the 2ecccaede9097f867284d352a881d8f226ba4fb7 is
> quite broken, and should be reverted.

I reverted it.  Sorry about that; it had a horrible typo (!= vs ==).  Does the 
attached (untested) patch work for you instead?  It fixes the typo, and also 
pacifies GCC so that GCC does not issue the bogus warning.

>From 7d298c5558df36cf0e6a46940ace042c52284264 Mon Sep 17 00:00:00 2001
From: Paul Eggert <eggert@cs.ucla.edu>
Date: Sun, 15 Mar 2015 17:38:10 -0700
Subject: [PATCH] * stdlib/setenv.c (__add_to_environ):

Dump core quickly if setenv (..., NULL, ...) is called.
This time, do it the right way, and pacify GCC with a pragma.
---
 ChangeLog       |  4 ++++
 stdlib/setenv.c | 18 +++++++++++++++++-
 2 files changed, 21 insertions(+), 1 deletion(-)

diff --git a/ChangeLog b/ChangeLog
index c856f79..e61cc17 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,5 +1,9 @@
 2015-03-15  Paul Eggert  <eggert@cs.ucla.edu>
 
+	* stdlib/setenv.c (__add_to_environ):
+	Dump core quickly if setenv (..., NULL, ...) is called.
+	This time, do it the right way, and pacify GCC with a pragma.
+
 	* stdlib/setenv.c (__add_to_environ): Revert previous change.
 
 2015-03-14  Andreas Schwab  <schwab@linux-m68k.org>
diff --git a/stdlib/setenv.c b/stdlib/setenv.c
index b60c4f0..184a8cd 100644
--- a/stdlib/setenv.c
+++ b/stdlib/setenv.c
@@ -19,6 +19,13 @@
 # include <config.h>
 #endif
 
+/* 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.  Do not use DIAG_IGNORE_NEEDS_COMMENT
+   here, as it's not defined yet.  */
+#pragma GCC diagnostic ignored "-Wmaybe-uninitialized"
+
 #include <errno.h>
 #if !_LIBC
 # if !defined errno && !defined HAVE_ERRNO_DECL
@@ -114,8 +121,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;
 
-- 
2.1.0