git/list[1] front-page[2] threads[3] people[4] search[5] about
 

[PATCH 2/6] run_external_diff: use an argv_array for the environment

From
Jeff King <peff@peff.net>
Date
Apr 19, 2014, 19:17 UTC
Message-ID
<20140419191725.GB8981@sigill.intra.peff.net>
In-Reply-To
<20140419191128.GA9266@sigill.intra.peff.net>

We currently use static buffers and a static array for formatting the environment passed to the external diff. There's nothing wrong in the code, but it is much easier to verify that it is correct if we use a dynamic argv_array.

Signed-off-by: Jeff King <peff@peff.net>
---
 diff.c | 14 +++++---------
 1 file changed, 5 insertions(+), 9 deletions(-)
diff --git a/diff.c b/diff.c
index b154284..760fc96 100644
--- a/diff.c
+++ b/diff.c
@@ -2904,11 +2904,9 @@ static void run_external_diff(const char *pgm,
 			      struct diff_options *o)
 {
 	struct argv_array argv = ARGV_ARRAY_INIT;
+	struct argv_array env = ARGV_ARRAY_INIT;
 	int retval;
 	struct diff_queue_struct *q = &diff_queued_diff;
-	const char *env[3] = { NULL };
-	char env_counter[50];
-	char env_total[50];
 
 	if (one && two) {
 		struct diff_tempfile *temp_one, *temp_two;
@@ -2933,15 +2931,13 @@ static void run_external_diff(const char *pgm,
 	}
 	fflush(NULL);
 
-	env[0] = env_counter;
-	snprintf(env_counter, sizeof(env_counter), "GIT_DIFF_PATH_COUNTER=%d",
-		 ++o->diff_path_counter);
-	env[1] = env_total;
-	snprintf(env_total, sizeof(env_total), "GIT_DIFF_PATH_TOTAL=%d", q->nr);
+	argv_array_pushf(&env, "GIT_DIFF_PATH_COUNTER=%d", ++o->diff_path_counter);
+	argv_array_pushf(&env, "GIT_DIFF_PATH_TOTAL=%d", q->nr);
 
-	retval = run_command_v_opt_cd_env(argv.argv, RUN_USING_SHELL, NULL, env);
+	retval = run_command_v_opt_cd_env(argv.argv, RUN_USING_SHELL, NULL, env.argv);
 	remove_tempfile();
 	argv_array_clear(&argv);
+	argv_array_clear(&env);
 	if (retval) {
 		fprintf(stderr, "external diff died, stopping at %s.\n", name);
 		exit(1);
-- 
1.9.1.656.ge8a0637
Previous: Eric SunshineNext: Jeff King
Message 6 of 10 in “run_external_diff cleanups”
  1. 0/6 run_external_diff cleanupsJeff King, Apr 19, 2014
  2. 1/6 run_external_diff: use an argv_array for the command lineJeff King, Apr 19, 2014
  3. Max LApr 19, 2014
  4. Jeff KingApr 20, 2014
  5. Eric SunshineApr 20, 2014
  6. 2/6 run_external_diff: use an argv_array for the environmentJeff King, Apr 19, 2014
  7. 3/6 run_external_diff: clean up error handlingJeff King, Apr 19, 2014
  8. 4/6 run_external_diff: drop fflush(NULL)Jeff King, Apr 19, 2014
  9. 5/6 run_external_diff: hoist common bits out of conditionalJeff King, Apr 19, 2014
  10. 6/6 run_external_diff: refactor cmdline setup logicJeff King, Apr 19, 2014

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.