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

[PATCH v3 3/9] run-command API users: use strvec_pushv(), not argv assignment

From
Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Date
Nov 25, 2021, 22:52 UTC
Message-ID
<patch-v3-3.9-595ff9a775d-20211125T224833Z-avarab@gmail.com>
In-Reply-To
<cover-v3-0.9-00000000000-20211125T224833Z-avarab@gmail.com>

Migrate those run-command API users that assign directly to the "argv" member to use a strvec_pushv() of "args" instead.

In these cases it did not make sense to further refactor these callers, e.g. daemon.c could be made to construct the arguments closer to handle(), but that would require moving the construction from its cmd_main() and pass "argv" through two intermediate functions.

It would be possible for a change like this to introduce a regression if we were doing:

      cp.argv = argv;
      argv[1] = "foo";
And changed the code, as is being done here, to:
      strvec_pushv(&cp.args, argv);
      argv[1] = "foo";

But as viewing this change with the "-W" flag reveals none of these functions modify variable that's being pushed afterwards in a way that would introduce such a logic error.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 add-patch.c                | 4 ++--
 daemon.c                   | 2 +-
 http-backend.c             | 2 +-
 http.c                     | 5 +++--
 remote-curl.c              | 2 +-
 run-command.c              | 2 +-
 t/helper/test-subprocess.c | 2 +-
 7 files changed, 10 insertions(+), 9 deletions(-)
diff --git a/add-patch.c b/add-patch.c
index 8c41cdfe39b..573eef0cc4a 100644
--- a/add-patch.c
+++ b/add-patch.c
@@ -413,7 +413,7 @@ static int parse_diff(struct add_p_state *s, const struct pathspec *ps)
 		strvec_push(&args, ps->items[i].original);
 
 	setup_child_process(s, &cp, NULL);
-	cp.argv = args.v;
+	strvec_pushv(&cp.args, args.v);
 	res = capture_command(&cp, plain, 0);
 	if (res) {
 		strvec_clear(&args);
@@ -431,7 +431,7 @@ static int parse_diff(struct add_p_state *s, const struct pathspec *ps)
 
 		setup_child_process(s, &colored_cp, NULL);
 		xsnprintf((char *)args.v[color_arg_index], 8, "--color");
-		colored_cp.argv = args.v;
+		strvec_pushv(&colored_cp.args, args.v);
 		colored = &s->colored;
 		res = capture_command(&colored_cp, colored, 0);
 		strvec_clear(&args);
diff --git a/daemon.c b/daemon.c
index b1fcbe0d6fa..8df21f2130c 100644
--- a/daemon.c
+++ b/daemon.c
@@ -922,7 +922,7 @@ static void handle(int incoming, struct sockaddr *addr, socklen_t addrlen)
 #endif
 	}
 
-	cld.argv = cld_argv.v;
+	strvec_pushv(&cld.args, cld_argv.v);
 	cld.in = incoming;
 	cld.out = dup(incoming);
 
diff --git a/http-backend.c b/http-backend.c
index 3d6e2ff17f8..4dd4d939f8a 100644
--- a/http-backend.c
+++ b/http-backend.c
@@ -480,7 +480,7 @@ static void run_service(const char **argv, int buffer_input)
 		strvec_pushf(&cld.env_array,
 			     "GIT_COMMITTER_EMAIL=%s@http.%s", user, host);
 
-	cld.argv = argv;
+	strvec_pushv(&cld.args, argv);
 	if (buffer_input || gzipped_request || req_len >= 0)
 		cld.in = -1;
 	cld.git_cmd = 1;
diff --git a/http.c b/http.c
index f92859f43fa..229da4d1488 100644
--- a/http.c
+++ b/http.c
@@ -2126,8 +2126,9 @@ int finish_http_pack_request(struct http_pack_request *preq)
 
 	ip.git_cmd = 1;
 	ip.in = tmpfile_fd;
-	ip.argv = preq->index_pack_args ? preq->index_pack_args
-					: default_index_pack_args;
+	strvec_pushv(&ip.args, preq->index_pack_args ?
+		     preq->index_pack_args :
+		     default_index_pack_args);
 
 	if (preq->preserve_index_pack_stdout)
 		ip.out = 0;
diff --git a/remote-curl.c b/remote-curl.c
index d69156312bd..0dabef2dd7c 100644
--- a/remote-curl.c
+++ b/remote-curl.c
@@ -1061,7 +1061,7 @@ static int rpc_service(struct rpc_state *rpc, struct discovery *heads,
 	client.in = -1;
 	client.out = -1;
 	client.git_cmd = 1;
-	client.argv = client_argv;
+	strvec_pushv(&client.args, client_argv);
 	if (start_command(&client))
 		exit(1);
 	write_or_die(client.in, preamble->buf, preamble->len);
diff --git a/run-command.c b/run-command.c
index f40df01c772..620a06ca2f5 100644
--- a/run-command.c
+++ b/run-command.c
@@ -1039,7 +1039,7 @@ int run_command_v_opt_cd_env_tr2(const char **argv, int opt, const char *dir,
 				 const char *const *env, const char *tr2_class)
 {
 	struct child_process cmd = CHILD_PROCESS_INIT;
-	cmd.argv = argv;
+	strvec_pushv(&cmd.args, argv);
 	cmd.no_stdin = opt & RUN_COMMAND_NO_STDIN ? 1 : 0;
 	cmd.git_cmd = opt & RUN_GIT_CMD ? 1 : 0;
 	cmd.stdout_to_stderr = opt & RUN_COMMAND_STDOUT_TO_STDERR ? 1 : 0;
diff --git a/t/helper/test-subprocess.c b/t/helper/test-subprocess.c
index 92b69de6352..ff22f2fa2c5 100644
--- a/t/helper/test-subprocess.c
+++ b/t/helper/test-subprocess.c
@@ -15,6 +15,6 @@ int cmd__subprocess(int argc, const char **argv)
 		argv++;
 	}
 	cp.git_cmd = 1;
-	cp.argv = (const char **)argv + 1;
+	strvec_pushv(&cp.args, (const char **)argv + 1);
 	return run_command(&cp);
 }
-- 
2.34.1.838.g779e9098efb
Previous: Eric SunshineNext: Ævar Arnfjörð Bjarmason
Message 50 of 71 in “pager: fix crash when pager program doesn't exist”
  1. pager: fix crash when pager program doesn't existEnzo Matsumiya, Nov 20, 2021
  2. Jeff KingNov 21, 2021
  3. Junio C HamanoNov 22, 2021
  4. Jeff KingNov 22, 2021
  5. Enzo MatsumiyaNov 22, 2021
  6. Junio C HamanoNov 22, 2021
  7. Enzo MatsumiyaNov 23, 2021
  8. Ævar Arnfjörð BjarmasonNov 24, 2021
  9. Jeff KingNov 24, 2021
  10. 0/5 run-command API: get rid of "argv"Ævar Arnfjörð Bjarmason, Nov 22, 2021
  11. 1/5 archive-tar: use our own cmd.buf in error messageÆvar Arnfjörð Bjarmason, Nov 22, 2021
  12. Junio C HamanoNov 22, 2021
  13. 2/5 upload-archive: use regular "struct child_process" patternÆvar Arnfjörð Bjarmason, Nov 22, 2021
  14. Jeff KingNov 22, 2021
  15. Ævar Arnfjörð BjarmasonNov 22, 2021
  16. Jeff KingNov 22, 2021
  17. Ævar Arnfjörð BjarmasonNov 22, 2021
  18. 3/5 run-command API users: use strvec_pushv(), not argv assignmentÆvar Arnfjörð Bjarmason, Nov 22, 2021
  19. Junio C HamanoNov 22, 2021
  20. Ævar Arnfjörð BjarmasonNov 22, 2021
  21. 5/5 run-command API: remove "argv" member, always use "args"Ævar Arnfjörð Bjarmason, Nov 22, 2021
  22. Jeff KingNov 22, 2021
  23. Ævar Arnfjörð BjarmasonNov 22, 2021
  24. Jeff KingNov 22, 2021
  25. 4/5 run-command API users: use strvec_pushl(), not argv constructionÆvar Arnfjörð Bjarmason, Nov 22, 2021
  26. Jeff KingNov 22, 2021
  27. Junio C HamanoNov 22, 2021
  28. Ævar Arnfjörð BjarmasonNov 22, 2021
  29. Jeff KingNov 22, 2021
  30. Ævar Arnfjörð BjarmasonNov 22, 2021
  31. 0/9 run-command API: get rid of "argv" and "env"Ævar Arnfjörð Bjarmason, Nov 23, 2021
  32. 1/9 worktree: remove redundant NULL-ing of "cp.argvÆvar Arnfjörð Bjarmason, Nov 23, 2021
  33. Eric SunshineNov 23, 2021
  34. Junio C HamanoNov 24, 2021
  35. Eric SunshineNov 24, 2021
  36. Eric SunshineNov 24, 2021
  37. Eric SunshineNov 24, 2021
  38. 2/9 upload-archive: use regular "struct child_process" patternÆvar Arnfjörð Bjarmason, Nov 23, 2021
  39. 3/9 run-command API users: use strvec_pushv(), not argv assignmentÆvar Arnfjörð Bjarmason, Nov 23, 2021
  40. 4/9 run-command tests: use strvec_pushv(), not argv assignmentÆvar Arnfjörð Bjarmason, Nov 23, 2021
  41. Eric SunshineNov 24, 2021
  42. 5/9 run-command API users: use strvec_pushl(), not argv constructionÆvar Arnfjörð Bjarmason, Nov 23, 2021
  43. 6/9 run-command API users: use strvec_push(), not argv constructionÆvar Arnfjörð Bjarmason, Nov 23, 2021
  44. 7/9 run-command API: remove "argv" member, always use "args"Ævar Arnfjörð Bjarmason, Nov 23, 2021
  45. 8/9 difftool: use "env_array" to simplify memory managementÆvar Arnfjörð Bjarmason, Nov 23, 2021
  46. 9/9 run-command API: remove "env" member, always use "env_array"Ævar Arnfjörð Bjarmason, Nov 23, 2021
  47. 0/9 run-command API: get rid of "argv" and "env"Ævar Arnfjörð Bjarmason, Nov 25, 2021
  48. 1/9 worktree: stop being overly intimate with run_command() internalsÆvar Arnfjörð Bjarmason, Nov 25, 2021
  49. Eric SunshineNov 26, 2021
  50. 3/9 run-command API users: use strvec_pushv(), not argv assignmentÆvar Arnfjörð Bjarmason, Nov 25, 2021
  51. 2/9 upload-archive: use regular "struct child_process" patternÆvar Arnfjörð Bjarmason, Nov 25, 2021
  52. 4/9 run-command tests: use strvec_pushv(), not argv assignmentÆvar Arnfjörð Bjarmason, Nov 25, 2021
  53. 6/9 run-command API users: use strvec_push(), not argv constructionÆvar Arnfjörð Bjarmason, Nov 25, 2021
  54. 5/9 run-command API users: use strvec_pushl(), not argv constructionÆvar Arnfjörð Bjarmason, Nov 25, 2021
  55. 7/9 run-command API: remove "argv" member, always use "args"Ævar Arnfjörð Bjarmason, Nov 25, 2021
  56. 8/9 difftool: use "env_array" to simplify memory managementÆvar Arnfjörð Bjarmason, Nov 25, 2021
  57. 9/9 run-command API: remove "env" member, always use "env_array"Ævar Arnfjörð Bjarmason, Nov 25, 2021
  58. Enzo MatsumiyaNov 22, 2021
  59. Ævar Arnfjörð BjarmasonNov 22, 2021
  60. Enzo MatsumiyaNov 22, 2021
  61. Ævar Arnfjörð BjarmasonNov 22, 2021
  62. Jeff KingNov 22, 2021
  63. Junio C HamanoNov 22, 2021
  64. Jeff KingNov 22, 2021
  65. Junio C HamanoNov 22, 2021
  66. Junio C HamanoNov 22, 2021
  67. Jeff KingNov 22, 2021
  68. Junio C HamanoNov 22, 2021
  69. Junio C HamanoNov 22, 2021
  70. Ævar Arnfjörð BjarmasonNov 22, 2021
  71. Jeff KingNov 22, 2021

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.