Re: [Win32] putenv modifications not inherited by child processed (Was: Heap corruption in putenv)

[Paul Eggert]

On 02/19/13 14:04, John W. Eaton wrote:
> and also to use _putenv in the normal case when the argument string to putenv 
> contains "VAR=VAL".

Ouch, thanks for catching that; it suggests further
fixes.  Could you please try this patch to gnulib instead?

>From ae287e04d310465ed2e8bd7df61b65094b66b4e4 Mon Sep 17 00:00:00 2001
From: Paul Eggert <address@hidden>
Date: Mon, 18 Feb 2013 19:38:13 -0800
Subject: [PATCH] putenv: port better to native Windows

* lib/putenv.c [(_WIN32 || __WIN32__) && ! __CYGWIN__]:
Define WIN32_LEAN_AND_MEAN and include <windows.h>.
(_unsetenv): Use _putenv if available.
(putenv): Temporarily set NAME=' ' rather than NAME='x' as that's
a bit less likely to cause damage.
(putenv) [(_WIN32 || __WIN32__) && ! __CYGWIN__]:
Fix the wrong value with SetEnvironmentVariable.
(putenv) [!HAVE__PUTENV]: Simplify and match the HAVE__PUTENV
code better.
---
 ChangeLog    | 11 +++++++++
 lib/putenv.c | 76 ++++++++++++++++++++++++++++++++++++++++--------------------
 2 files changed, 62 insertions(+), 25 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index 4f3a198..67fd17d 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,5 +1,16 @@
 2013-02-19  Paul Eggert  <address@hidden>
 
+       putenv: port better to native Windows
+       * lib/putenv.c [(_WIN32 || __WIN32__) && ! __CYGWIN__]:
+       Define WIN32_LEAN_AND_MEAN and include <windows.h>.
+       (_unsetenv): Use _putenv if available.
+       (putenv): Temporarily set NAME=' ' rather than NAME='x' as that's
+       a bit less likely to cause damage.
+       (putenv) [(_WIN32 || __WIN32__) && ! __CYGWIN__]:
+       Fix the wrong value with SetEnvironmentVariable.
+       (putenv) [!HAVE__PUTENV]: Simplify and match the HAVE__PUTENV
+       code better.
+
        strtod: support coreutils better
        * lib/strtod.c (underlying_strtod): Just invoke the underlying strtod.
        HAVE_RAW_DECL_STRTOD might not be correct in coreutils, which
diff --git a/lib/putenv.c b/lib/putenv.c
index 0ec392f..4c65e78 100644
--- a/lib/putenv.c
+++ b/lib/putenv.c
@@ -34,6 +34,11 @@
 #include <string.h>
 #include <unistd.h>
 
+#if (defined _WIN32 || defined __WIN32__) && ! defined __CYGWIN__
+# define WIN32_LEAN_AND_MEAN
+# include <windows.h>
+#endif
+
 #if _LIBC
 # if HAVE_GNU_LD
 #  define environ __environ
@@ -67,6 +72,21 @@ _unsetenv (const char *name)
 
   len = strlen (name);
 
+#if HAVE__PUTENV
+  {
+    int putenv_result, putenv_errno;
+    char *name_ = malloc (len + 2);
+    memcpy (name_, name, len);
+    name_[len] = '=';
+    name_[len + 1] = 0;
+    putenv_result = _putenv (name_);
+    putenv_errno = errno;
+    free (name_);
+    __set_errno (putenv_errno);
+    return putenv_result;
+  }
+#else
+
   LOCK;
 
   ep = environ;
@@ -87,6 +107,7 @@ _unsetenv (const char *name)
   UNLOCK;
 
   return 0;
+#endif
 }
 
 
@@ -95,9 +116,8 @@ _unsetenv (const char *name)
 int
 putenv (char *string)
 {
-  const char *const name_end = strchr (string, '=');
-  register size_t size;
-  register char **ep;
+  const char *name_end = strchr (string, '=');
+  char **ep;
 
   if (name_end == NULL)
     {
@@ -105,16 +125,6 @@ putenv (char *string)
       return _unsetenv (string);
     }
 
-  size = 0;
-  for (ep = environ; *ep != NULL; ++ep)
-    if (!strncmp (*ep, string, name_end - string) &&
-        (*ep)[name_end - string] == '=')
-      break;
-    else
-      ++size;
-
-  if (*ep == NULL)
-    {
 #if HAVE__PUTENV
       /* Rely on _putenv to allocate the new environment.  If other
          parts of the application use _putenv, the !HAVE__PUTENV code
@@ -123,15 +133,15 @@ putenv (char *string)
         return _putenv (string);
       else
         {
-          /* _putenv ("NAME=") unsets NAME, so invoke _putenv ("NAME=x")
+          /* _putenv ("NAME=") unsets NAME, so invoke _putenv ("NAME= ")
              to allocate the environ vector and then replace the new
              entry with "NAME=".  */
           int putenv_result, putenv_errno;
-          char *name_x = malloc (name_end - string + sizeof "=x");
+          char *name_x = malloc (name_end - string + sizeof "= ");
           if (!name_x)
             return -1;
           memcpy (name_x, string, name_end - string + 1);
-          name_x[name_end - string + 1] = 'x';
+          name_x[name_end - string + 1] = ' ';
           name_x[name_end - string + 2] = 0;
           putenv_result = _putenv (name_x);
           putenv_errno = errno;
@@ -141,26 +151,42 @@ putenv (char *string)
                 *ep = string;
                 break;
               }
+# if (defined _WIN32 || defined __WIN32__) && ! defined __CYGWIN__
+          if (putenv_result == 0)
+            {
+              /* _putenv propagated "NAME= " into the subprocess environment;
+                 fix that by calling SetEnvironmentVariable directly.  */
+              name_x[name_end - string] = 0;
+              putenv_result = SetEnvironmentVariable (name_x, "") ? 0 : -1;
+              putenv_errno = ENOMEM; /* ENOMEM is the only way to fail.  */
+            }
+# endif
           free (name_x);
           __set_errno (putenv_errno);
           return putenv_result;
         }
 #else
+  for (ep = environ; *ep; ep++)
+    if (strncmp (*ep, string, name_end - string) == 0
+        && (*ep)[name_end - string] == '=')
+      break;
+
+  if (*ep)
+    *ep = string;
+  else
+    {
       static char **last_environ = NULL;
-      char **new_environ = (char **) malloc ((size + 2) * sizeof (char *));
-      if (new_environ == NULL)
+      size_t size = ep - environ;
+      char **new_environ = malloc ((size + 2) * sizeof *new_environ);
+      if (! new_environ)
         return -1;
-      (void) memcpy ((void *) new_environ, (void *) environ,
-                     size * sizeof (char *));
-      new_environ[size] = (char *) string;
-      new_environ[size + 1] = NULL;
+      new_environ[0] = string;
+      memcpy (new_environ + 1, environ, (size + 1) * sizeof *new_environ);
       free (last_environ);
       last_environ = new_environ;
       environ = new_environ;
-#endif
     }
-  else
-    *ep = string;
+#endif
 
   return 0;
 }
-- 
1.7.11.7