diff options
author | Karsten Blees <blees@dcon.de> | 2014-07-17 19:38:01 +0400 |
---|---|---|
committer | Junio C Hamano <gitster@pobox.com> | 2014-07-21 20:32:49 +0400 |
commit | 77734da241f97607eea8f2bb55e0e19937e918d4 (patch) | |
tree | 9df58d07e94ea309e2ef3f6043b3ec714d4d35ce | |
parent | df0e998c31a6d91b905b1ad5b3ff89435ba29313 (diff) |
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>
-rw-r--r-- | compat/mingw.c | 76 | ||||
-rw-r--r-- | compat/mingw.h | 8 | ||||
-rw-r--r-- | run-command.c | 10 |
3 files changed, 30 insertions, 64 deletions
diff --git a/compat/mingw.c b/compat/mingw.c index 8c5cf903ba..789a0ec6dc 100644 --- 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 828d97760c..3851857c2d 100644 --- 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 be07d4ad33..82dd0853a1 100644 --- 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); |