win32: always use safe API calls to manipulate environment

It turns out that with the new toolchain safe API calls work on all
all platforms.  Even the original code from commit fa147bd7e works
on Windows XP when built with the new tools.

- Remove the unsafe environment manipulation via the environ array
- Microsoft's putenv takes a copy of its argument so the string
  can be freed
- Rewrite some routines and add more error checking

5 files changed, 26 insertions, 118 deletions

diff --git a/Config.in b/Config.in
index ba7ba1026..dc3a6e09a 100644
--- a/Config.in
+++ b/Config.in
@@ -134,16 +134,6 @@ config GLOBBING
           the BusyBox binary to handle wildcard expansion using the C runtime
           set this to 'Y'.
 
-config SAFE_ENV
-        bool "Manipulate the environment through safe API calls"
-        default n
-        depends on PLATFORM_MINGW32
-        help
-          Enable this option to use safe API calls when clearing environment
-          variables.  This is necessary if BusyBox is to run on ReactOS or
-          64-bit Windows.  The default is 'N', which must be used if BusyBox
-          is to run on Windows XP.
-
 config PAM
         bool "Support PAM (Pluggable Authentication Modules)"
         default n
diff --git a/configs/mingw32_defconfig b/configs/mingw32_defconfig
index 4adc4471e..5ed8cd41f 100644
--- a/configs/mingw32_defconfig
+++ b/configs/mingw32_defconfig
@@ -20,7 +20,6 @@ CONFIG_FEATURE_VERBOSE_USAGE=y
 CONFIG_FEATURE_COMPRESS_USAGE=y
 CONFIG_LFS=y
 # CONFIG_GLOBBING is not set
-# CONFIG_SAFE_ENV is not set
 # CONFIG_PAM is not set
 # CONFIG_FEATURE_DEVPTS is not set
 # CONFIG_FEATURE_UTMP is not set
diff --git a/configs/mingw64_defconfig b/configs/mingw64_defconfig
index fdc755d93..2c189700a 100644
--- a/configs/mingw64_defconfig
+++ b/configs/mingw64_defconfig
@@ -20,7 +20,6 @@ CONFIG_FEATURE_VERBOSE_USAGE=y
 CONFIG_FEATURE_COMPRESS_USAGE=y
 CONFIG_LFS=y
 # CONFIG_GLOBBING is not set
-CONFIG_SAFE_ENV=y
 # CONFIG_PAM is not set
 # CONFIG_FEATURE_DEVPTS is not set
 # CONFIG_FEATURE_UTMP is not set
diff --git a/include/mingw.h b/include/mingw.h
index 87abd077b..6f69913d6 100644
--- a/include/mingw.h
+++ b/include/mingw.h
@@ -195,16 +195,10 @@ char *mingw_mktemp(char *template);
 int mkstemp(char *template);
 char *realpath(const char *path, char *resolved_path);
 int setenv(const char *name, const char *value, int replace);
-#if ENABLE_SAFE_ENV
 int unsetenv(const char *env);
-#else
-void unsetenv(const char *env);
-#endif
 
 #define getenv mingw_getenv
-#if ENABLE_SAFE_ENV
 #define putenv mingw_putenv
-#endif
 #define mktemp mingw_mktemp
 
 /*
diff --git a/win32/env.c b/win32/env.c
index eab3c1708..2837c0720 100644
--- a/win32/env.c
+++ b/win32/env.c
@@ -1,19 +1,8 @@
 #include "libbb.h"
 
-static int lookup_env(char **env, const char *name, size_t nmln)
-{
-        int i;
-
-        for (i = 0; env[i]; i++) {
-                if (0 == strncmp(env[i], name, nmln)
-                    && '=' == env[i][nmln])
-                        /* matches */
-                        return i;
-        }
-        return -1;
-}
-
 #undef getenv
+#undef putenv
+
 char *mingw_getenv(const char *name)
 {
         char *result = getenv(name);
@@ -29,70 +18,21 @@ char *mingw_getenv(const char *name)
 int setenv(const char *name, const char *value, int replace)
 {
         int out;
-        size_t namelen, valuelen;
         char *envstr;
 
-        if (!name || !value) return -1;
+        if (!name || !*name || strchr(name, '=') || !value) return -1;
         if (!replace) {
-                char *oldval = NULL;
-                oldval = getenv(name);
-                if (oldval) return 0;
+                if (getenv(name)) return 0;
         }
 
-        namelen = strlen(name);
-        valuelen = strlen(value);
-        envstr = malloc((namelen + valuelen + 2));
-        if (!envstr) return -1;
-
-        memcpy(envstr, name, namelen);
-        envstr[namelen] = '=';
-        memcpy(envstr + namelen + 1, value, valuelen);
-        envstr[namelen + valuelen + 1] = 0;
-
-        out = putenv(envstr);
-        /* putenv(3) makes the argument string part of the environment,
-         * and changing that string modifies the environment --- which
-         * means we do not own that storage anymore.  Do not free
-         * envstr.
-         */
+        envstr = xasprintf("%s=%s", name, value);
+        out = mingw_putenv(envstr);
+        free(envstr);
 
         return out;
 }
 
 /*
- * If name contains '=', then sets the variable, otherwise it unsets it
- */
-char **env_setenv(char **env, const char *name)
-{
-        char *eq = strchrnul(name, '=');
-        int i = lookup_env(env, name, eq-name);
-
-        if (i < 0) {
-                if (*eq) {
-                        for (i = 0; env[i]; i++)
-                                ;
-                        env = xrealloc(env, (i+2)*sizeof(*env));
-                        env[i] = xstrdup(name);
-                        env[i+1] = NULL;
-                }
-        }
-        else {
-                free(env[i]);
-                if (*eq)
-                        env[i] = xstrdup(name);
-                else {
-                        for (; env[i]; i++)
-                                env[i] = env[i+1];
-#if !ENABLE_SAFE_ENV
-                        SetEnvironmentVariable(name, NULL);
-#endif
-                }
-        }
-        return env;
-}
-
-#if ENABLE_SAFE_ENV
-/*
  * Removing an environment variable with WIN32 putenv requires an argument
  * like "NAME="; glibc omits the '='.  The implementations of unsetenv and
  * clearenv allow for this.
@@ -100,28 +40,34 @@ char **env_setenv(char **env, const char *name)
  * It isn't possible to create an environment variable with an empty value
  * using WIN32 putenv.
  */
-#undef putenv
-int unsetenv(const char *env)
+int unsetenv(const char *name)
 {
-        char *name;
+        char *envstr;
         int ret;
 
-        name = xmalloc(strlen(env)+2);
-        strcat(strcpy(name, env), "=");
-        ret = putenv(name);
-        free(name);
+        if (!name || !*name || strchr(name, '=') ) {
+                return -1;
+        }
+
+        envstr = xmalloc(strlen(name)+2);
+        strcat(strcpy(envstr, name), "=");
+        ret = putenv(envstr);
+        free(envstr);
 
         return ret;
 }
 
 int clearenv(void)
 {
-        char *name, *s;
-
-        while ( environ && *environ ) {
-                if ( (s=strchr(*environ, '=')) != NULL ) {
-                        name = xstrndup(*environ, s-*environ+1);
-                        putenv(name);
+        char *envp, *name, *s;
+
+        while ( environ && (envp=*environ) ) {
+                if ( (s=strchr(envp, '=')) != NULL ) {
+                        name = xstrndup(envp, s-envp+1);
+                        if ( putenv(name) == -1 ) {
+                                free(name);
+                                return -1;
+                        }
                         free(name);
                 }
                 else {
@@ -146,23 +92,3 @@ int mingw_putenv(const char *env)
         /* can't set empty value */
         return 0;
 }
-#else
-void unsetenv(const char *env)
-{
-        env_setenv(environ, env);
-}
-
-int clearenv(void)
-{
-        char **env = environ;
-        if (!env)
-                return 0;
-        while (*env) {
-                free(*env);
-                env++;
-        }
-        free(env);
-        environ = NULL;
-        return 0;
-}
-#endif