git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: "Ævar Arnfjörð Bjarmason" <avarab@gmail.com>
To: git@vger.kernel.org
Cc: "Junio C Hamano" <gitster@pobox.com>, "Jeff King" <peff@peff.net>,
	"Enzo Matsumiya" <ematsumiya@suse.de>,
	"Eric Sunshine" <sunshine@sunshineco.com>,
	"Ævar Arnfjörð Bjarmason" <avarab@gmail.com>
Subject: [PATCH v3 3/9] run-command API users: use strvec_pushv(), not argv assignment
Date: Thu, 25 Nov 2021 23:52:18 +0100	[thread overview]
Message-ID: <patch-v3-3.9-595ff9a775d-20211125T224833Z-avarab@gmail.com> (raw)
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


  parent reply	other threads:[~2021-11-25 22:54 UTC|newest]

Thread overview: 71+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-11-20 19:40 [PATCH v2] pager: fix crash when pager program doesn't exist Enzo Matsumiya
2021-11-21 18:37 ` Jeff King
2021-11-22  2:10   ` Junio C Hamano
2021-11-22  4:35     ` Jeff King
2021-11-22 14:52       ` Enzo Matsumiya
2021-11-22 17:05         ` Junio C Hamano
2021-11-23 16:40           ` Enzo Matsumiya
2021-11-24  1:55             ` Ævar Arnfjörð Bjarmason
2021-11-24 15:51               ` Jeff King
2021-11-22 16:04       ` [PATCH 0/5] run-command API: get rid of "argv" Ævar Arnfjörð Bjarmason
2021-11-22 16:04         ` [PATCH 1/5] archive-tar: use our own cmd.buf in error message Ævar Arnfjörð Bjarmason
2021-11-22 21:04           ` Junio C Hamano
2021-11-22 16:04         ` [PATCH 2/5] upload-archive: use regular "struct child_process" pattern Ævar Arnfjörð Bjarmason
2021-11-22 17:02           ` Jeff King
2021-11-22 20:53           ` Ævar Arnfjörð Bjarmason
2021-11-22 21:10             ` Jeff King
2021-11-22 21:36               ` Ævar Arnfjörð Bjarmason
2021-11-22 16:04         ` [PATCH 3/5] run-command API users: use strvec_pushv(), not argv assignment Ævar Arnfjörð Bjarmason
2021-11-22 21:19           ` Junio C Hamano
2021-11-22 21:30             ` Ævar Arnfjörð Bjarmason
2021-11-22 16:04         ` [PATCH 4/5] run-command API users: use strvec_pushl(), not argv construction Ævar Arnfjörð Bjarmason
2021-11-22 16:04         ` [PATCH 5/5] run-command API: remove "argv" member, always use "args" Ævar Arnfjörð Bjarmason
2021-11-22 17:32           ` Jeff King
2021-11-22 18:19             ` Ævar Arnfjörð Bjarmason
2021-11-22 18:47               ` Jeff King
2021-11-22 17:52         ` [PATCH 0/5] run-command API: get rid of "argv" Jeff King
2021-11-22 18:11           ` Junio C Hamano
2021-11-22 18:33             ` Ævar Arnfjörð Bjarmason
2021-11-22 18:49               ` Jeff King
2021-11-22 18:26           ` Ævar Arnfjörð Bjarmason
2021-11-23 12:06         ` [PATCH v2 0/9] run-command API: get rid of "argv" and "env" Ævar Arnfjörð Bjarmason
2021-11-23 12:06           ` [PATCH v2 1/9] worktree: remove redundant NULL-ing of "cp.argv Ævar Arnfjörð Bjarmason
2021-11-23 15:26             ` Eric Sunshine
2021-11-24  1:54               ` Junio C Hamano
2021-11-24  6:00                 ` Eric Sunshine
2021-11-24  6:12                   ` Eric Sunshine
2021-11-24  5:44               ` Eric Sunshine
2021-11-23 12:06           ` [PATCH v2 2/9] upload-archive: use regular "struct child_process" pattern Ævar Arnfjörð Bjarmason
2021-11-23 12:06           ` [PATCH v2 3/9] run-command API users: use strvec_pushv(), not argv assignment Ævar Arnfjörð Bjarmason
2021-11-23 12:06           ` [PATCH v2 4/9] run-command tests: " Ævar Arnfjörð Bjarmason
2021-11-24  1:33             ` Eric Sunshine
2021-11-23 12:06           ` [PATCH v2 5/9] run-command API users: use strvec_pushl(), not argv construction Ævar Arnfjörð Bjarmason
2021-11-23 12:06           ` [PATCH v2 6/9] run-command API users: use strvec_push(), " Ævar Arnfjörð Bjarmason
2021-11-23 12:06           ` [PATCH v2 7/9] run-command API: remove "argv" member, always use "args" Ævar Arnfjörð Bjarmason
2021-11-23 12:06           ` [PATCH v2 8/9] difftool: use "env_array" to simplify memory management Ævar Arnfjörð Bjarmason
2021-11-23 12:06           ` [PATCH v2 9/9] run-command API: remove "env" member, always use "env_array" Ævar Arnfjörð Bjarmason
2021-11-25 22:52           ` [PATCH v3 0/9] run-command API: get rid of "argv" and "env" Ævar Arnfjörð Bjarmason
2021-11-25 22:52             ` [PATCH v3 1/9] worktree: stop being overly intimate with run_command() internals Ævar Arnfjörð Bjarmason
2021-11-26  9:48               ` Eric Sunshine
2021-11-25 22:52             ` [PATCH v3 2/9] upload-archive: use regular "struct child_process" pattern Ævar Arnfjörð Bjarmason
2021-11-25 22:52             ` Ævar Arnfjörð Bjarmason [this message]
2021-11-25 22:52             ` [PATCH v3 4/9] run-command tests: use strvec_pushv(), not argv assignment Ævar Arnfjörð Bjarmason
2021-11-25 22:52             ` [PATCH v3 5/9] run-command API users: use strvec_pushl(), not argv construction Ævar Arnfjörð Bjarmason
2021-11-25 22:52             ` [PATCH v3 6/9] run-command API users: use strvec_push(), " Ævar Arnfjörð Bjarmason
2021-11-25 22:52             ` [PATCH v3 7/9] run-command API: remove "argv" member, always use "args" Ævar Arnfjörð Bjarmason
2021-11-25 22:52             ` [PATCH v3 8/9] difftool: use "env_array" to simplify memory management Ævar Arnfjörð Bjarmason
2021-11-25 22:52             ` [PATCH v3 9/9] run-command API: remove "env" member, always use "env_array" Ævar Arnfjörð Bjarmason
2021-11-22 15:31     ` [PATCH v2] pager: fix crash when pager program doesn't exist Enzo Matsumiya
2021-11-22 16:22       ` Ævar Arnfjörð Bjarmason
2021-11-22 16:46         ` Enzo Matsumiya
2021-11-22 17:10           ` Ævar Arnfjörð Bjarmason
2021-11-22 17:41             ` Jeff King
2021-11-22 18:00             ` Junio C Hamano
2021-11-22 18:26               ` Jeff King
2021-11-22 17:55           ` Junio C Hamano
2021-11-22 18:19             ` Junio C Hamano
2021-11-22 18:37               ` Jeff King
2021-11-22 20:39                 ` Junio C Hamano
2021-11-22 17:08         ` Junio C Hamano
2021-11-22 18:35           ` Ævar Arnfjörð Bjarmason
2021-11-22 16:30       ` Jeff King

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: http://vger.kernel.org/majordomo-info.html

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=patch-v3-3.9-595ff9a775d-20211125T224833Z-avarab@gmail.com \
    --to=avarab@gmail.com \
    --cc=ematsumiya@suse.de \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=peff@peff.net \
    --cc=sunshine@sunshineco.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).