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>
Karsten Blees committed
Jul 17, 2014 at 17:38 UTC
77734da241f97607eea8f2bb55e0e19937e918d4
3 files changed
+30
-64
compat/mingw.c
+26
-50
@@ -899,6 +899,8 @@ static char *path_lookup(const char *cmd, char **path, int exe_only)
899
return prog;
900
}
901
902
+static char **do_putenv(char **env, const char *name, int free_old);
903
+
904
static int compareenv(const void *a, const void *b)
905
{
906
char *const *ea = a;
@@ -907,21 +909,30 @@ static int compareenv(const void *a, const void *b)
909
}
910
911
/*
910
- * Create environment block suitable for CreateProcess.
912
+ * Create environment block suitable for CreateProcess. Merges current
913
+ * process environment and the supplied environment changes.
914
*/
912
-static wchar_t *make_environment_block(char **env)
915
+static wchar_t *make_environment_block(char **deltaenv)
916
{
917
wchar_t *wenvblk = NULL;
918
int count = 0;
919
char **e, **tmpenv;
920
int size = 0, wenvsz = 0, wenvpos = 0;
921
919
- for (e = env; *e; e++)
922
+ while (environ[count])
923
count++;
924
922
- /* environment must be sorted */
925
+ /* copy the environment */
926
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++;
936
qsort(tmpenv, count, sizeof(*tmpenv), compareenv);
937
938
/* create environment block from temporary environment */
@@ -944,7 +955,7 @@ struct pinfo_t {
955
static struct pinfo_t *pinfo = NULL;
956
CRITICAL_SECTION pinfo_cs;
957
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,
959
const char *dir,
960
int prepend_cmd, int fhin, int fhout, int fherr)
961
{
@@ -1012,8 +1023,7 @@ static pid_t mingw_spawnve_fd(const char *cmd, const char **argv, char **env,
1023
xutftowcs(wargs, args.buf, 2 * args.len + 1);
1024
strbuf_release(&args);
1025
1015
- if (env)
1016
- wenvblk = make_environment_block(env);
1026
+ wenvblk = make_environment_block(deltaenv);
1027
1028
memset(&pi, 0, sizeof(pi));
1029
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,
1061
1062
static pid_t mingw_spawnv(const char *cmd, const char **argv, int prepend_cmd)
1063
{
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);
1065
}
1066
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,
1068
const char *dir,
1069
int fhin, int fhout, int fherr)
1070
{
@@ -1078,14 +1088,14 @@ pid_t mingw_spawnvpe(const char *cmd, const char **argv, char **env,
1088
pid = -1;
1089
}
1090
else {
1081
- pid = mingw_spawnve_fd(iprog, argv, env, dir, 1,
1091
+ pid = mingw_spawnve_fd(iprog, argv, deltaenv, dir, 1,
1092
fhin, fhout, fherr);
1093
free(iprog);
1094
}
1095
argv[0] = argv0;
1096
}
1097
else
1088
- pid = mingw_spawnve_fd(prog, argv, env, dir, 0,
1098
+ pid = mingw_spawnve_fd(prog, argv, deltaenv, dir, 0,
1099
fhin, fhout, fherr);
1100
free(prog);
1101
}
@@ -1182,27 +1192,6 @@ int mingw_kill(pid_t pid, int sig)
1192
return -1;
1193
}
1194
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
-
1195
static int lookupenv(char **env, const char *name, size_t nmln)
1196
{
1197
int i;
@@ -1218,7 +1207,7 @@ static int lookupenv(char **env, const char *name, size_t nmln)
1207
/*
1208
* If name contains '=', then sets the variable, otherwise it unsets it
1209
*/
1221
-static char **do_putenv(char **env, const char *name)
1210
+static char **do_putenv(char **env, const char *name, int free_old)
1211
{
1212
char *eq = strchrnul(name, '=');
1213
int i = lookupenv(env, name, eq-name);
@@ -1233,7 +1222,8 @@ static char **do_putenv(char **env, const char *name)
1222
}
1223
}
1224
else {
1236
- free(env[i]);
1225
+ if (free_old)
1226
+ free(env[i]);
1227
if (*eq)
1228
env[i] = (char*) name;
1229
else
@@ -1243,20 +1233,6 @@ static char **do_putenv(char **env, const char *name)
1233
return env;
1234
}
1235
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
-
1236
#undef getenv
1237
char *mingw_getenv(const char *name)
1238
{
@@ -1272,7 +1248,7 @@ char *mingw_getenv(const char *name)
1248
1249
int mingw_putenv(const char *namevalue)
1250
{
1275
- environ = do_putenv(environ, namevalue);
1251
+ environ = do_putenv(environ, namevalue, 1);
1252
return 0;
1253
}
1254
compat/mingw.h
+2
-6
@@ -357,12 +357,8 @@ int mingw_offset_1st_component(const char *path);
357
void mingw_open_html(const char *path);
358
#define open_html mingw_open_html
359
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
362
363
/**
364
* Converts UTF-8 encoded string to UTF-16LE.
run-command.c
+2
-8
@@ -454,7 +454,6 @@ fail_pipe:
454
{
455
int fhin = 0, fhout = 1, fherr = 2;
456
const char **sargv = cmd->argv;
457
- char **env = environ;
457
458
if (cmd->no_stdin)
459
fhin = open("/dev/null", O_RDWR);
@@ -479,24 +478,19 @@ fail_pipe:
478
else if (cmd->out > 1)
479
fhout = dup(cmd->out);
480
482
- if (cmd->env)
483
- env = make_augmented_environ(cmd->env);
484
-
481
if (cmd->git_cmd)
482
cmd->argv = prepare_git_cmd(cmd->argv);
483
else if (cmd->use_shell)
484
cmd->argv = prepare_shell_cmd(cmd->argv);
485
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);
488
failed_errno = errno;
489
if (cmd->pid < 0 && (!cmd->silent_exec_failure || errno != ENOENT))
490
error("cannot spawn %s: %s", cmd->argv[0], strerror(errno));
491
if (cmd->clean_on_exit && cmd->pid >= 0)
492
mark_child_for_cleanup(cmd->pid);
493
498
- if (cmd->env)
499
- free_environ(env);
494
if (cmd->git_cmd)
495
free(cmd->argv);
496