threads / patch / 20904

patch, 2 partsstart_command: do not clobber cmd->env on Windows code path

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

## tl;dr

3 messages between Sep 11, 2009 and Sep 12, 2009. Diffs are folded; open one to read it.

replies: 2people: 2as markdown or json

Johannes Sixt· Sep 11, 2009, 17:40 UTC · lore

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(-)
Show changes to 3 files +17 −9

compat/mingw.c, compat/mingw.h, run-command.c

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
Johannes Sixt· Sep 11, 2009, 17:45 UTC · re: Johannes Sixt · lore

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

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(-)
Show changes to pager.c +4 −2
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

← back to recent threads