run-command API users: use strvec_pushl(), not argv construction

Change a pattern of hardcoding an "argv" array size, populating it and assigning to the "argv" member of "struct child_process" to instead use "strvec_pushl()" to add data to the "args" member. This implements the same behavior as before in fewer lines of code, and moves us further towards being able to remove the "argv" member in a subsequent commit. Since we've entirely removed the "argv" variable(s) we can be sure that no potential logic errors of the type discussed in a preceding commit are being introduced here, i.e. ones where the local "argv" was being modified after the assignment to "struct child_process"'s "argv". Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Ævar Arnfjörð Bjarmason committed Nov 25, 2021 at 23:52 UTC 2b7098936c9e91d527aa53b8d4af0b25d7e912b4
10 files changed +29 -61
builtin/add.c
+2 -5
@@ -302,15 +302,11 @@ int interactive_add(const char **argv, const char *prefix, int patch)
302 static int edit_patch(int argc, const char **argv, const char *prefix)
303 {
304 char *file = git_pathdup("ADD_EDIT.patch");
305 - const char *apply_argv[] = { "apply", "--recount", "--cached",
306 - NULL, NULL };
305 struct child_process child = CHILD_PROCESS_INIT;
306 struct rev_info rev;
307 int out;
308 struct stat st;
309
312 - apply_argv[3] = file;
313 -
310 git_config(git_diff_basic_config, NULL); /* no "diff" UI options */
311
312 if (read_cache() < 0)
@@ -338,7 +334,8 @@ static int edit_patch(int argc, const char **argv, const char *prefix)
334 die(_("Empty patch. Aborted."));
335
336 child.git_cmd = 1;
341 - child.argv = apply_argv;
337 + strvec_pushl(&child.args, "apply", "--recount", "--cached", file,
338 + NULL);
339 if (run_command(&child))
340 die(_("Could not apply '%s'"), file);
341
builtin/fsck.c
+4 -8
@@ -944,15 +944,13 @@ int cmd_fsck(int argc, const char **argv, const char *prefix)
944
945 if (the_repository->settings.core_commit_graph) {
946 struct child_process commit_graph_verify = CHILD_PROCESS_INIT;
947 - const char *verify_argv[] = { "commit-graph", "verify", NULL, NULL, NULL };
947
948 prepare_alt_odb(the_repository);
949 for (odb = the_repository->objects->odb; odb; odb = odb->next) {
950 child_process_init(&commit_graph_verify);
952 - commit_graph_verify.argv = verify_argv;
951 commit_graph_verify.git_cmd = 1;
954 - verify_argv[2] = "--object-dir";
955 - verify_argv[3] = odb->path;
952 + strvec_pushl(&commit_graph_verify.args, "commit-graph",
953 + "verify", "--object-dir", odb->path, NULL);
954 if (run_command(&commit_graph_verify))
955 errors_found |= ERROR_COMMIT_GRAPH;
956 }
@@ -960,15 +958,13 @@ int cmd_fsck(int argc, const char **argv, const char *prefix)
958
959 if (the_repository->settings.core_multi_pack_index) {
960 struct child_process midx_verify = CHILD_PROCESS_INIT;
963 - const char *midx_argv[] = { "multi-pack-index", "verify", NULL, NULL, NULL };
961
962 prepare_alt_odb(the_repository);
963 for (odb = the_repository->objects->odb; odb; odb = odb->next) {
964 child_process_init(&midx_verify);
968 - midx_verify.argv = midx_argv;
965 midx_verify.git_cmd = 1;
970 - midx_argv[2] = "--object-dir";
971 - midx_argv[3] = odb->path;
966 + strvec_pushl(&midx_verify.args, "multi-pack-index",
967 + "verify", "--object-dir", odb->path, NULL);
968 if (run_command(&midx_verify))
969 errors_found |= ERROR_MULTI_PACK_INDEX;
970 }
builtin/help.c
+1 -2
@@ -212,11 +212,10 @@ static int check_emacsclient_version(void)
212 {
213 struct strbuf buffer = STRBUF_INIT;
214 struct child_process ec_process = CHILD_PROCESS_INIT;
215 - const char *argv_ec[] = { "emacsclient", "--version", NULL };
215 int version;
216
217 /* emacsclient prints its version number on stderr */
219 - ec_process.argv = argv_ec;
218 + strvec_pushl(&ec_process.args, "emacsclient", "--version", NULL);
219 ec_process.err = -1;
220 ec_process.stdout_to_stderr = 1;
221 if (start_command(&ec_process))
builtin/merge.c
+1 -2
@@ -310,10 +310,9 @@ static int save_state(struct object_id *stash)
310 int len;
311 struct child_process cp = CHILD_PROCESS_INIT;
312 struct strbuf buffer = STRBUF_INIT;
313 - const char *argv[] = {"stash", "create", NULL};
313 int rc = -1;
314
316 - cp.argv = argv;
315 + strvec_pushl(&cp.args, "stash", "create", NULL);
316 cp.out = -1;
317 cp.git_cmd = 1;
318
builtin/notes.c
+2 -3
@@ -134,14 +134,13 @@ static void copy_obj_to_fd(int fd, const struct object_id *oid)
134
135 static void write_commented_object(int fd, const struct object_id *object)
136 {
137 - const char *show_args[5] =
138 - {"show", "--stat", "--no-notes", oid_to_hex(object), NULL};
137 struct child_process show = CHILD_PROCESS_INIT;
138 struct strbuf buf = STRBUF_INIT;
139 struct strbuf cbuf = STRBUF_INIT;
140
141 /* Invoke "git show --stat --no-notes $object" */
144 - show.argv = show_args;
142 + strvec_pushl(&show.args, "show", "--stat", "--no-notes",
143 + oid_to_hex(object), NULL);
144 show.no_stdin = 1;
145 show.out = -1;
146 show.err = 0;
builtin/receive-pack.c
+13 -25
@@ -1370,22 +1370,10 @@ static const char *push_to_deploy(unsigned char *sha1,
1370 struct strvec *env,
1371 const char *work_tree)
1372 {
1373 - const char *update_refresh[] = {
1374 - "update-index", "-q", "--ignore-submodules", "--refresh", NULL
1375 - };
1376 - const char *diff_files[] = {
1377 - "diff-files", "--quiet", "--ignore-submodules", "--", NULL
1378 - };
1379 - const char *diff_index[] = {
1380 - "diff-index", "--quiet", "--cached", "--ignore-submodules",
1381 - NULL, "--", NULL
1382 - };
1383 - const char *read_tree[] = {
1384 - "read-tree", "-u", "-m", NULL, NULL
1385 - };
1373 struct child_process child = CHILD_PROCESS_INIT;
1374
1388 - child.argv = update_refresh;
1375 + strvec_pushl(&child.args, "update-index", "-q", "--ignore-submodules",
1376 + "--refresh", NULL);
1377 child.env = env->v;
1378 child.dir = work_tree;
1379 child.no_stdin = 1;
@@ -1396,7 +1384,8 @@ static const char *push_to_deploy(unsigned char *sha1,
1384
1385 /* run_command() does not clean up completely; reinitialize */
1386 child_process_init(&child);
1399 - child.argv = diff_files;
1387 + strvec_pushl(&child.args, "diff-files", "--quiet",
1388 + "--ignore-submodules", "--", NULL);
1389 child.env = env->v;
1390 child.dir = work_tree;
1391 child.no_stdin = 1;
@@ -1405,11 +1394,12 @@ static const char *push_to_deploy(unsigned char *sha1,
1394 if (run_command(&child))
1395 return "Working directory has unstaged changes";
1396
1408 - /* diff-index with either HEAD or an empty tree */
1409 - diff_index[4] = head_has_history() ? "HEAD" : empty_tree_oid_hex();
1410 -
1397 child_process_init(&child);
1412 - child.argv = diff_index;
1398 + strvec_pushl(&child.args, "diff-index", "--quiet", "--cached",
1399 + "--ignore-submodules",
1400 + /* diff-index with either HEAD or an empty tree */
1401 + head_has_history() ? "HEAD" : empty_tree_oid_hex(),
1402 + "--", NULL);
1403 child.env = env->v;
1404 child.no_stdin = 1;
1405 child.no_stdout = 1;
@@ -1418,9 +1408,9 @@ static const char *push_to_deploy(unsigned char *sha1,
1408 if (run_command(&child))
1409 return "Working directory has staged changes";
1410
1421 - read_tree[3] = hash_to_hex(sha1);
1411 child_process_init(&child);
1423 - child.argv = read_tree;
1412 + strvec_pushl(&child.args, "read-tree", "-u", "-m", hash_to_hex(sha1),
1413 + NULL);
1414 child.env = env->v;
1415 child.dir = work_tree;
1416 child.no_stdin = 1;
@@ -2575,16 +2565,14 @@ int cmd_receive_pack(int argc, const char **argv, const char *prefix)
2565 run_update_post_hook(commands);
2566 string_list_clear(&push_options, 0);
2567 if (auto_gc) {
2578 - const char *argv_gc_auto[] = {
2579 - "gc", "--auto", "--quiet", NULL,
2580 - };
2568 struct child_process proc = CHILD_PROCESS_INIT;
2569
2570 proc.no_stdin = 1;
2571 proc.stdout_to_stderr = 1;
2572 proc.err = use_sideband ? -1 : 0;
2573 proc.git_cmd = proc.close_object_store = 1;
2587 - proc.argv = argv_gc_auto;
2574 + strvec_pushl(&proc.args, "gc", "--auto", "--quiet",
2575 + NULL);
2576
2577 if (!start_command(&proc)) {
2578 if (use_sideband)
builtin/replace.c
+1 -2
@@ -258,11 +258,10 @@ static int import_object(struct object_id *oid, enum object_type type,
258 return error_errno(_("unable to open %s for reading"), filename);
259
260 if (!raw && type == OBJ_TREE) {
261 - const char *argv[] = { "mktree", NULL };
261 struct child_process cmd = CHILD_PROCESS_INIT;
262 struct strbuf result = STRBUF_INIT;
263
265 - cmd.argv = argv;
264 + strvec_push(&cmd.args, "mktree");
265 cmd.git_cmd = 1;
266 cmd.in = fd;
267 cmd.out = -1;
editor.c
+1 -3
@@ -55,7 +55,6 @@ static int launch_specified_editor(const char *editor, const char *path,
55
56 if (strcmp(editor, ":")) {
57 struct strbuf realpath = STRBUF_INIT;
58 - const char *args[] = { editor, NULL, NULL };
58 struct child_process p = CHILD_PROCESS_INIT;
59 int ret, sig;
60 int print_waiting_for_editor = advice_enabled(ADVICE_WAITING_FOR_EDITOR) && isatty(2);
@@ -77,9 +76,8 @@ static int launch_specified_editor(const char *editor, const char *path,
76 }
77
78 strbuf_realpath(&realpath, path, 1);
80 - args[1] = realpath.buf;
79
82 - p.argv = args;
80 + strvec_pushl(&p.args, editor, realpath.buf, NULL);
81 p.env = env;
82 p.use_shell = 1;
83 p.trace2_child_class = "editor";
sequencer.c
+3 -7
@@ -1164,18 +1164,14 @@ static int run_rewrite_hook(const struct object_id *oldoid,
1164 const struct object_id *newoid)
1165 {
1166 struct child_process proc = CHILD_PROCESS_INIT;
1167 - const char *argv[3];
1167 int code;
1168 struct strbuf sb = STRBUF_INIT;
1169 + const char *hook_path = find_hook("post-rewrite");
1170
1171 - argv[0] = find_hook("post-rewrite");
1172 - if (!argv[0])
1171 + if (!hook_path)
1172 return 0;
1173
1175 - argv[1] = "amend";
1176 - argv[2] = NULL;
1177 -
1178 - proc.argv = argv;
1174 + strvec_pushl(&proc.args, hook_path, "amend", NULL);
1175 proc.in = -1;
1176 proc.stdout_to_stderr = 1;
1177 proc.trace2_hook_name = "post-rewrite";
upload-pack.c
+1 -4
@@ -596,14 +596,11 @@ static int do_reachable_revlist(struct child_process *cmd,
596 struct object_array *reachable,
597 enum allow_uor allow_uor)
598 {
599 - static const char *argv[] = {
600 - "rev-list", "--stdin", NULL,
601 - };
599 struct object *o;
600 FILE *cmd_in = NULL;
601 int i;
602
606 - cmd->argv = argv;
603 + strvec_pushl(&cmd->args, "rev-list", "--stdin", NULL);
604 cmd->git_cmd = 1;
605 cmd->no_stderr = 1;
606 cmd->in = -1;