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

[PATCH 1/3] execv_dashed_external: use child_process struct

From
Jeff King <peff@peff.net>
Date
Jan 7, 2017, 01:16 UTC
Message-ID
<20170107011624.hnvu2m5jaosebadf@sigill.intra.peff.net>
In-Reply-To
<20170107011445.3e4fv6vdtimrwhgv@sigill.intra.peff.net>

When we run a dashed external, we use the one-liner run_command_v_opt() to do so. Let's switch to using a child_process struct, which has two advantages:

  1. We can drop all of the allocation and cleanup code for
     building our custom argv array, and just rely on the
     builtin argv_array (at the minor cost of doing a few
     extra mallocs).
  2. We have access to the complete range of child_process
     options, not just the ones that the "_opt()" form can
     forward.
Signed-off-by: Jeff King <peff@peff.net>
---

It's possible people may disagree with reason (2), and we should add a new RUN_WAIT_AFTER_CLEAN flag in the final patch. IMHO the whole run_command_v_opt() thing is a good sign that you should be switching to a child_process struct. :)

 git.c | 25 +++++++------------------
 1 file changed, 7 insertions(+), 18 deletions(-)
diff --git a/git.c b/git.c
index dce529fcbf..d0e04d5c97 100644
--- a/git.c
+++ b/git.c
@@ -575,8 +575,7 @@ static void handle_builtin(int argc, const char **argv)
 
 static void execv_dashed_external(const char **argv)
 {
-	struct strbuf cmd = STRBUF_INIT;
-	const char *tmp;
+	struct child_process cmd = CHILD_PROCESS_INIT;
 	int status;
 
 	if (get_super_prefix())
@@ -586,30 +585,20 @@ static void execv_dashed_external(const char **argv)
 		use_pager = check_pager_config(argv[0]);
 	commit_pager_choice();
 
-	strbuf_addf(&cmd, "git-%s", argv[0]);
+	argv_array_pushf(&cmd.args, "git-%s", argv[0]);
+	argv_array_pushv(&cmd.args, argv + 1);
+	cmd.clean_on_exit = 1;
+	cmd.silent_exec_failure = 1;
 
-	/*
-	 * argv[0] must be the git command, but the argv array
-	 * belongs to the caller, and may be reused in
-	 * subsequent loop iterations. Save argv[0] and
-	 * restore it on error.
-	 */
-	tmp = argv[0];
-	argv[0] = cmd.buf;
-
-	trace_argv_printf(argv, "trace: exec:");
+	trace_argv_printf(cmd.args.argv, "trace: exec:");
 
 	/*
 	 * if we fail because the command is not found, it is
 	 * OK to return. Otherwise, we just pass along the status code.
 	 */
-	status = run_command_v_opt(argv, RUN_SILENT_EXEC_FAILURE | RUN_CLEAN_ON_EXIT);
+	status = run_command(&cmd);
 	if (status >= 0 || errno != ENOENT)
 		exit(status);
-
-	argv[0] = tmp;
-
-	strbuf_release(&cmd);
 }
 
 static int run_argv(int *argcp, const char ***argv)
-- 
2.11.0.527.gfef230ca76
Previous: Jeff KingNext: Jeff King
Message 12 of 19 in “Regression: Ctrl-c from the pager in an alias exits it”
  1. Trygve AabergeJan 5, 2017
  2. Jeff KingJan 6, 2017
  3. Jeff KingJan 6, 2017
  4. Jeff KingJan 6, 2017
  5. Jeff KingJan 6, 2017
  6. Trygve AabergeJan 6, 2017
  7. Johannes SixtJan 6, 2017
  8. Jeff KingJan 6, 2017
  9. Johannes SixtJan 6, 2017
  10. Jeff KingJan 6, 2017
  11. 0/3 fix ^C killing pager when running aliasJeff King, Jan 7, 2017
  12. 1/3 execv_dashed_external: use child_process structJeff King, Jan 7, 2017
  13. 2/3 execv_dashed_external: stop exiting with negative codeJeff King, Jan 7, 2017
  14. 3/3 execv_dashed_external: wait for child on signal deathJeff King, Jan 7, 2017
  15. Johannes SixtJan 7, 2017
  16. Jeff KingJan 7, 2017
  17. Duy NguyenJan 7, 2017
  18. Jacob KellerJan 7, 2017
  19. Jacob KellerJan 7, 2017

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.