Skip to content

Commit 77734da

Browse files
kbleesgitster
kblees
authored and
gitster
committed
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>
1 parent df0e998 commit 77734da

File tree

3 files changed

+30
-64
lines changed

3 files changed

+30
-64
lines changed

‎compat/mingw.c‎

Lines changed: 26 additions & 50 deletions
Original file line numberDiff line numberDiff line change
@@ -899,6 +899,8 @@ static char *path_lookup(const char *cmd, char **path, int exe_only)
899899
return prog;
900900
}
901901

902+
static char **do_putenv(char **env, const char *name, int free_old);
903+
902904
static int compareenv(const void *a, const void *b)
903905
{
904906
char *const *ea = a;
@@ -907,21 +909,30 @@ static int compareenv(const void *a, const void *b)
907909
}
908910

909911
/*
910-
* Create environment block suitable for CreateProcess.
912+
* Create environment block suitable for CreateProcess. Merges current
913+
* process environment and the supplied environment changes.
911914
*/
912-
static wchar_t *make_environment_block(char **env)
915+
static wchar_t *make_environment_block(char **deltaenv)
913916
{
914917
wchar_t *wenvblk = NULL;
915918
int count = 0;
916919
char **e, **tmpenv;
917920
int size = 0, wenvsz = 0, wenvpos = 0;
918921

919-
for (e = env; *e; e++)
922+
while (environ[count])
920923
count++;
921924

922-
/* environment must be sorted */
925+
/* copy the environment */
923926
tmpenv = xmalloc(sizeof(*tmpenv) * (count + 1));
924-
memcpy(tmpenv, env, sizeof(*tmpenv) * (count + 1));
927+
memcpy(tmpenv, environ, sizeof(*tmpenv) * (count + 1));
928+
929+
/* merge supplied environment changes into the temporary environment */
930+
for (e = deltaenv; e && *e; e++)
931+
tmpenv = do_putenv(tmpenv, *e, 0);
932+
933+
/* environment must be sorted */
934+
for (count = 0; tmpenv[count]; )
935+
count++;
925936
qsort(tmpenv, count, sizeof(*tmpenv), compareenv);
926937

927938
/* create environment block from temporary environment */
@@ -944,7 +955,7 @@ struct pinfo_t {
944955
static struct pinfo_t *pinfo = NULL;
945956
CRITICAL_SECTION pinfo_cs;
946957

947-
static pid_t mingw_spawnve_fd(const char *cmd, const char **argv, char **env,
958+
static pid_t mingw_spawnve_fd(const char *cmd, const char **argv, char **deltaenv,
948959
const char *dir,
949960
int prepend_cmd, int fhin, int fhout, int fherr)
950961
{
@@ -1012,8 +1023,7 @@ static pid_t mingw_spawnve_fd(const char *cmd, const char **argv, char **env,
10121023
xutftowcs(wargs, args.buf, 2 * args.len + 1);
10131024
strbuf_release(&args);
10141025

1015-
if (env)
1016-
wenvblk = make_environment_block(env);
1026+
wenvblk = make_environment_block(deltaenv);
10171027

10181028
memset(&pi, 0, sizeof(pi));
10191029
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,
10511061

10521062
static pid_t mingw_spawnv(const char *cmd, const char **argv, int prepend_cmd)
10531063
{
1054-
return mingw_spawnve_fd(cmd, argv, environ, NULL, prepend_cmd, 0, 1, 2);
1064+
return mingw_spawnve_fd(cmd, argv, NULL, NULL, prepend_cmd, 0, 1, 2);
10551065
}
10561066

1057-
pid_t mingw_spawnvpe(const char *cmd, const char **argv, char **env,
1067+
pid_t mingw_spawnvpe(const char *cmd, const char **argv, char **deltaenv,
10581068
const char *dir,
10591069
int fhin, int fhout, int fherr)
10601070
{
@@ -1078,14 +1088,14 @@ pid_t mingw_spawnvpe(const char *cmd, const char **argv, char **env,
10781088
pid = -1;
10791089
}
10801090
else {
1081-
pid = mingw_spawnve_fd(iprog, argv, env, dir, 1,
1091+
pid = mingw_spawnve_fd(iprog, argv, deltaenv, dir, 1,
10821092
fhin, fhout, fherr);
10831093
free(iprog);
10841094
}
10851095
argv[0] = argv0;
10861096
}
10871097
else
1088-
pid = mingw_spawnve_fd(prog, argv, env, dir, 0,
1098+
pid = mingw_spawnve_fd(prog, argv, deltaenv, dir, 0,
10891099
fhin, fhout, fherr);
10901100
free(prog);
10911101
}
@@ -1182,27 +1192,6 @@ int mingw_kill(pid_t pid, int sig)
11821192
return -1;
11831193
}
11841194

1185-
static char **copy_environ(void)
1186-
{
1187-
char **env;
1188-
int i = 0;
1189-
while (environ[i])
1190-
i++;
1191-
env = xmalloc((i+1)*sizeof(*env));
1192-
for (i = 0; environ[i]; i++)
1193-
env[i] = xstrdup(environ[i]);
1194-
env[i] = NULL;
1195-
return env;
1196-
}
1197-
1198-
void free_environ(char **env)
1199-
{
1200-
int i;
1201-
for (i = 0; env[i]; i++)
1202-
free(env[i]);
1203-
free(env);
1204-
}
1205-
12061195
static int lookupenv(char **env, const char *name, size_t nmln)
12071196
{
12081197
int i;
@@ -1218,7 +1207,7 @@ static int lookupenv(char **env, const char *name, size_t nmln)
12181207
/*
12191208
* If name contains '=', then sets the variable, otherwise it unsets it
12201209
*/
1221-
static char **do_putenv(char **env, const char *name)
1210+
static char **do_putenv(char **env, const char *name, int free_old)
12221211
{
12231212
char *eq = strchrnul(name, '=');
12241213
int i = lookupenv(env, name, eq-name);
@@ -1233,7 +1222,8 @@ static char **do_putenv(char **env, const char *name)
12331222
}
12341223
}
12351224
else {
1236-
free(env[i]);
1225+
if (free_old)
1226+
free(env[i]);
12371227
if (*eq)
12381228
env[i] = (char*) name;
12391229
else
@@ -1243,20 +1233,6 @@ static char **do_putenv(char **env, const char *name)
12431233
return env;
12441234
}
12451235

1246-
/*
1247-
* Copies global environ and adjusts variables as specified by vars.
1248-
*/
1249-
char **make_augmented_environ(const char *const *vars)
1250-
{
1251-
char **env = copy_environ();
1252-
1253-
while (*vars) {
1254-
const char *v = *vars++;
1255-
env = do_putenv(env, strchr(v, '=') ? xstrdup(v) : v);
1256-
}
1257-
return env;
1258-
}
1259-
12601236
#undef getenv
12611237
char *mingw_getenv(const char *name)
12621238
{
@@ -1272,7 +1248,7 @@ char *mingw_getenv(const char *name)
12721248

12731249
int mingw_putenv(const char *namevalue)
12741250
{
1275-
environ = do_putenv(environ, namevalue);
1251+
environ = do_putenv(environ, namevalue, 1);
12761252
return 0;
12771253
}
12781254

‎compat/mingw.h‎

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -357,12 +357,8 @@ int mingw_offset_1st_component(const char *path);
357357
void mingw_open_html(const char *path);
358358
#define open_html mingw_open_html
359359

360-
/*
361-
* helpers
362-
*/
363-
364-
char **make_augmented_environ(const char *const *vars);
365-
void free_environ(char **env);
360+
void mingw_mark_as_git_dir(const char *dir);
361+
#define mark_as_git_dir mingw_mark_as_git_dir
366362

367363
/**
368364
* Converts UTF-8 encoded string to UTF-16LE.

‎run-command.c‎

Lines changed: 2 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -454,7 +454,6 @@ int start_command(struct child_process *cmd)
454454
{
455455
int fhin = 0, fhout = 1, fherr = 2;
456456
const char **sargv = cmd->argv;
457-
char **env = environ;
458457

459458
if (cmd->no_stdin)
460459
fhin = open("/dev/null", O_RDWR);
@@ -479,24 +478,19 @@ int start_command(struct child_process *cmd)
479478
else if (cmd->out > 1)
480479
fhout = dup(cmd->out);
481480

482-
if (cmd->env)
483-
env = make_augmented_environ(cmd->env);
484-
485481
if (cmd->git_cmd)
486482
cmd->argv = prepare_git_cmd(cmd->argv);
487483
else if (cmd->use_shell)
488484
cmd->argv = prepare_shell_cmd(cmd->argv);
489485

490-
cmd->pid = mingw_spawnvpe(cmd->argv[0], cmd->argv, env, cmd->dir,
491-
fhin, fhout, fherr);
486+
cmd->pid = mingw_spawnvpe(cmd->argv[0], cmd->argv, (char**) cmd->env,
487+
cmd->dir, fhin, fhout, fherr);
492488
failed_errno = errno;
493489
if (cmd->pid < 0 && (!cmd->silent_exec_failure || errno != ENOENT))
494490
error("cannot spawn %s: %s", cmd->argv[0], strerror(errno));
495491
if (cmd->clean_on_exit && cmd->pid >= 0)
496492
mark_child_for_cleanup(cmd->pid);
497493

498-
if (cmd->env)
499-
free_environ(env);
500494
if (cmd->git_cmd)
501495
free(cmd->argv);
502496

0 commit comments

Comments
 (0)