Win32: don't copy the environment twice when spawning child processes

When spawning child processes via start_command(), the environment and all
environment entries are copied twice. First by make_augmented_environ /
copy_environ to merge with child_process.env. Then a second time by
make_environment_block to create a sorted environment block string as
required by CreateProcess.

Move the merge logic to make_environment_block so that we only need to copy
the environment once. This changes semantics of the env parameter: it now
expects a delta (such as child_process.env) rather than a full environment.
This is not a problem as the parameter is only used by start_command()
(all other callers previously passed char **environ, and now pass NULL).

The merge logic no longer xstrdup()s the environment strings, so do_putenv
must not free them. Add a parameter to distinguish this from normal putenv.

Remove the now unused make_augmented_environ / free_environ API.

Signed-off-by: Karsten Blees <blees@dcon.de>
Signed-off-by: Stepan Kasal <kasal@ucw.cz>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

diff --git a/compat/mingw.c b/compat/mingw.c
index 8c5cf90..789a0ec 100644 (file)
--- a/compat/mingw.c
+++ b/compat/mingw.c
@@ -899,6 +899,8 @@ static char *path_lookup(const char *cmd, char **path, int exe_only)
        return prog;
 }
 
+static char **do_putenv(char **env, const char *name, int free_old);
+
 static int compareenv(const void *a, const void *b)
 {
        char *const *ea = a;
@@ -907,21 +909,30 @@ static int compareenv(const void *a, const void *b)
 }
 
 /*
- * Create environment block suitable for CreateProcess.
+ * Create environment block suitable for CreateProcess. Merges current
+ * process environment and the supplied environment changes.
  */
-static wchar_t *make_environment_block(char **env)
+static wchar_t *make_environment_block(char **deltaenv)
 {
        wchar_t *wenvblk = NULL;
        int count = 0;
        char **e, **tmpenv;
        int size = 0, wenvsz = 0, wenvpos = 0;
 
-       for (e = env; *e; e++)
+       while (environ[count])
                count++;
 
-       /* environment must be sorted */
+       /* copy the environment */
        tmpenv = xmalloc(sizeof(*tmpenv) * (count + 1));
-       memcpy(tmpenv, env, sizeof(*tmpenv) * (count + 1));
+       memcpy(tmpenv, environ, sizeof(*tmpenv) * (count + 1));
+
+       /* merge supplied environment changes into the temporary environment */
+       for (e = deltaenv; e && *e; e++)
+               tmpenv = do_putenv(tmpenv, *e, 0);
+
+       /* environment must be sorted */
+       for (count = 0; tmpenv[count]; )
+               count++;
        qsort(tmpenv, count, sizeof(*tmpenv), compareenv);
 
        /* create environment block from temporary environment */
@@ -944,7 +955,7 @@ struct pinfo_t {
 static struct pinfo_t *pinfo = NULL;
 CRITICAL_SECTION pinfo_cs;
 
-static pid_t mingw_spawnve_fd(const char *cmd, const char **argv, char **env,
+static pid_t mingw_spawnve_fd(const char *cmd, const char **argv, char **deltaenv,
                              const char *dir,
                              int prepend_cmd, int fhin, int fhout, int fherr)
 {
@@ -1012,8 +1023,7 @@ static pid_t mingw_spawnve_fd(const char *cmd, const char **argv, char **env,
        xutftowcs(wargs, args.buf, 2 * args.len + 1);
        strbuf_release(&args);
 
-       if (env)
-               wenvblk = make_environment_block(env);
+       wenvblk = make_environment_block(deltaenv);
 
        memset(&pi, 0, sizeof(pi));
        ret = CreateProcessW(wcmd, wargs, NULL, NULL, TRUE, flags,
@@ -1051,10 +1061,10 @@ static pid_t mingw_spawnve_fd(const char *cmd, const char **argv, char **env,
 
 static pid_t mingw_spawnv(const char *cmd, const char **argv, int prepend_cmd)
 {
-       return mingw_spawnve_fd(cmd, argv, environ, NULL, prepend_cmd, 0, 1, 2);
+       return mingw_spawnve_fd(cmd, argv, NULL, NULL, prepend_cmd, 0, 1, 2);
 }
 
-pid_t mingw_spawnvpe(const char *cmd, const char **argv, char **env,
+pid_t mingw_spawnvpe(const char *cmd, const char **argv, char **deltaenv,
                     const char *dir,
                     int fhin, int fhout, int fherr)
 {
@@ -1078,14 +1088,14 @@ pid_t mingw_spawnvpe(const char *cmd, const char **argv, char **env,
                                pid = -1;
                        }
                        else {
-                               pid = mingw_spawnve_fd(iprog, argv, env, dir, 1,
+                               pid = mingw_spawnve_fd(iprog, argv, deltaenv, dir, 1,
                                                       fhin, fhout, fherr);
                                free(iprog);
                        }
                        argv[0] = argv0;
                }
                else
-                       pid = mingw_spawnve_fd(prog, argv, env, dir, 0,
+                       pid = mingw_spawnve_fd(prog, argv, deltaenv, dir, 0,
                                               fhin, fhout, fherr);
                free(prog);
        }
@@ -1182,27 +1192,6 @@ int mingw_kill(pid_t pid, int sig)
        return -1;
 }
 
-static char **copy_environ(void)
-{
-       char **env;
-       int i = 0;
-       while (environ[i])
-               i++;
-       env = xmalloc((i+1)*sizeof(*env));
-       for (i = 0; environ[i]; i++)
-               env[i] = xstrdup(environ[i]);
-       env[i] = NULL;
-       return env;
-}
-
-void free_environ(char **env)
-{
-       int i;
-       for (i = 0; env[i]; i++)
-               free(env[i]);
-       free(env);
-}
-
 static int lookupenv(char **env, const char *name, size_t nmln)
 {
        int i;
@@ -1218,7 +1207,7 @@ static int lookupenv(char **env, const char *name, size_t nmln)
 /*
  * If name contains '=', then sets the variable, otherwise it unsets it
  */
-static char **do_putenv(char **env, const char *name)
+static char **do_putenv(char **env, const char *name, int free_old)
 {
        char *eq = strchrnul(name, '=');
        int i = lookupenv(env, name, eq-name);
@@ -1233,7 +1222,8 @@ static char **do_putenv(char **env, const char *name)
                }
        }
        else {
-               free(env[i]);
+               if (free_old)
+                       free(env[i]);
                if (*eq)
                        env[i] = (char*) name;
                else
@@ -1243,20 +1233,6 @@ static char **do_putenv(char **env, const char *name)
        return env;
 }
 
-/*
- * Copies global environ and adjusts variables as specified by vars.
- */
-char **make_augmented_environ(const char *const *vars)
-{
-       char **env = copy_environ();
-
-       while (*vars) {
-               const char *v = *vars++;
-               env = do_putenv(env, strchr(v, '=') ? xstrdup(v) : v);
-       }
-       return env;
-}
-
 #undef getenv
 char *mingw_getenv(const char *name)
 {
@@ -1272,7 +1248,7 @@ char *mingw_getenv(const char *name)
 
 int mingw_putenv(const char *namevalue)
 {
-       environ = do_putenv(environ, namevalue);
+       environ = do_putenv(environ, namevalue, 1);
        return 0;
 }
 

diff --git a/compat/mingw.h b/compat/mingw.h
index 828d977..3851857 100644 (file)
--- a/compat/mingw.h
+++ b/compat/mingw.h
@@ -357,12 +357,8 @@ int mingw_offset_1st_component(const char *path);
 void mingw_open_html(const char *path);
 #define open_html mingw_open_html
 
-/*
- * helpers
- */
-
-char **make_augmented_environ(const char *const *vars);
-void free_environ(char **env);
+void mingw_mark_as_git_dir(const char *dir);
+#define mark_as_git_dir mingw_mark_as_git_dir
 
 /**
  * Converts UTF-8 encoded string to UTF-16LE.

diff --git a/run-command.c b/run-command.c
index be07d4a..82dd085 100644 (file)
--- a/run-command.c
+++ b/run-command.c
@@ -454,7 +454,6 @@ fail_pipe:
 {
        int fhin = 0, fhout = 1, fherr = 2;
        const char **sargv = cmd->argv;
-       char **env = environ;
 
        if (cmd->no_stdin)
                fhin = open("/dev/null", O_RDWR);
@@ -479,24 +478,19 @@ fail_pipe:
        else if (cmd->out > 1)
                fhout = dup(cmd->out);
 
-       if (cmd->env)
-               env = make_augmented_environ(cmd->env);
-
        if (cmd->git_cmd)
                cmd->argv = prepare_git_cmd(cmd->argv);
        else if (cmd->use_shell)
                cmd->argv = prepare_shell_cmd(cmd->argv);
 
-       cmd->pid = mingw_spawnvpe(cmd->argv[0], cmd->argv, env, cmd->dir,
-                                 fhin, fhout, fherr);
+       cmd->pid = mingw_spawnvpe(cmd->argv[0], cmd->argv, (char**) cmd->env,
+                       cmd->dir, fhin, fhout, fherr);
        failed_errno = errno;
        if (cmd->pid < 0 && (!cmd->silent_exec_failure || errno != ENOENT))
                error("cannot spawn %s: %s", cmd->argv[0], strerror(errno));
        if (cmd->clean_on_exit && cmd->pid >= 0)
                mark_child_for_cleanup(cmd->pid);
 
-       if (cmd->env)
-               free_environ(env);
        if (cmd->git_cmd)
                free(cmd->argv);