[PATCH 1/2] start_command: do not clobber cmd->env on Windows code path

[PATCH 1/2] start_command: do not clobber cmd->env on Windows code path

[Johannes Sixt]

Previously, it would not be possible to call start_command twice for the
same struct child_process that has env set.

The fix is achieved by moving the loop that modifies the environment block
into a helper function. This also allows us to make two other helper
functions static.

Signed-off-by: Johannes Sixt <j6t@kdbg.org>
---
 We don't call start_command twice in a row anywhere in git.git, but
 msysgit has a patch that does, and with the next patch the buglet would
 be triggered.

 Even after this patch, other members of *cmd are clobbered, so it could
 be argued that this patch is unnecessary, but at least the Windows code
 path now keeps the same members that the Unix code path keeps, and it
 makes start_command easier to read by moving a loop into a helper.

 -- Hannes

 compat/mingw.c |   16 ++++++++++++++--
 compat/mingw.h |    3 +--
 run-command.c  |    7 ++-----
 3 files changed, 17 insertions(+), 9 deletions(-)

diff --git a/compat/mingw.c b/compat/mingw.c
index bed4178..36ef8d3 100644
--- a/compat/mingw.c
+++ b/compat/mingw.c
@@ -824,7 +824,7 @@ void mingw_execvp(const char *cmd, char *const *argv)
 	free_path_split(path);
 }
 
-char **copy_environ()
+static char **copy_environ(void)
 {
 	char **env;
 	int i = 0;
@@ -861,7 +861,7 @@ static int lookup_env(char **env, const char *name, size_t nmln)
 /*
  * If name contains '=', then sets the variable, otherwise it unsets it
  */
-char **env_setenv(char **env, const char *name)
+static char **env_setenv(char **env, const char *name)
 {
 	char *eq = strchrnul(name, '=');
 	int i = lookup_env(env, name, eq-name);
@@ -886,6 +886,18 @@ char **env_setenv(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)
+		env = env_setenv(env, *vars++);
+	return env;
+}
+
 /* this is the first function to call into WS_32; initialize it */
 #undef gethostbyname
 struct hostent *mingw_gethostbyname(const char *host)
diff --git a/compat/mingw.h b/compat/mingw.h
index 948de66..c43917c 100644
--- a/compat/mingw.h
+++ b/compat/mingw.h
@@ -222,9 +222,8 @@ void mingw_open_html(const char *path);
  * helpers
  */
 
-char **copy_environ(void);
+char **make_augmented_environ(const char *const *vars);
 void free_environ(char **env);
-char **env_setenv(char **env, const char *name);
 
 /*
  * A replacement of main() that ensures that argv[0] has a path
diff --git a/run-command.c b/run-command.c
index f3e7abb..ac314a5 100644
--- a/run-command.c
+++ b/run-command.c
@@ -173,11 +173,8 @@ fail_pipe:
 
 	if (cmd->dir)
 		die("chdir in start_command() not implemented");
-	if (cmd->env) {
-		env = copy_environ();
-		for (; *cmd->env; cmd->env++)
-			env = env_setenv(env, *cmd->env);
-	}
+	if (cmd->env)
+		env = make_augmented_environ(cmd->env);
 
 	if (cmd->git_cmd) {
 		cmd->argv = prepare_git_cmd(cmd->argv);
-- 
1.6.5.rc0.28.gfb9b

[PATCH 2/2] pager: set LESS=FRSX also on Windows

[Johannes Sixt]

Previously, this environment variable was set in the pager_preexec
callback, which is conditionally-compiled only on Unix, because it is not,
and cannot be, called on Windows.

With this patch the env member of struct child_process is used to set
the environment variable, which also works on Windows.

Noticed by Alexey Borzenkov.

Signed-off-by: Johannes Sixt <j6t@kdbg.org>
---
 Alexey,

 this will result in a conflict when you apply it to your msysgit because
 there is an extra patch that shuffled the setup of pager_process around.

 -- Hannes

 pager.c |    6 ++++--
 1 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/pager.c b/pager.c
index 4921843..f416d38 100644
--- a/pager.c
+++ b/pager.c
@@ -21,8 +21,6 @@ static void pager_preexec(void)
 	FD_ZERO(&in);
 	FD_SET(0, &in);
 	select(1, &in, NULL, &in, NULL);
-
-	setenv("LESS", "FRSX", 0);
 }
 #endif
 
@@ -70,6 +68,10 @@ void setup_pager(void)
 	pager_argv[2] = pager;
 	pager_process.argv = pager_argv;
 	pager_process.in = -1;
+	if (!getenv("LESS")) {
+		static const char *env[] = { "LESS=FRSX", NULL };
+		pager_process.env = env;
+	}
 #ifndef __MINGW32__
 	pager_process.preexec_cb = pager_preexec;
 #endif
-- 
1.6.5.rc0.28.gfb9b

Re: [PATCH 1/2] start_command: do not clobber cmd->env on Windows code path

[Junio C Hamano]

Thanks; both patches applied.