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>

diff --git a/builtin/add.c b/builtin/add.c
index ef6b619c45ed9e44266a04f425f6523d4d1da657..a010b2c325ffbec07fbd6b90a59f6582640b81ef 100644 (file)
--- a/builtin/add.c
+++ b/builtin/add.c
@@ -302,15 +302,11 @@ int interactive_add(const char **argv, const char *prefix, int patch)
 static int edit_patch(int argc, const char **argv, const char *prefix)
 {
        char *file = git_pathdup("ADD_EDIT.patch");
-       const char *apply_argv[] = { "apply", "--recount", "--cached",
-               NULL, NULL };
        struct child_process child = CHILD_PROCESS_INIT;
        struct rev_info rev;
        int out;
        struct stat st;
 
-       apply_argv[3] = file;
-
        git_config(git_diff_basic_config, NULL); /* no "diff" UI options */
 
        if (read_cache() < 0)
@@ -338,7 +334,8 @@ static int edit_patch(int argc, const char **argv, const char *prefix)
                die(_("Empty patch. Aborted."));
 
        child.git_cmd = 1;
-       child.argv = apply_argv;
+       strvec_pushl(&child.args, "apply", "--recount", "--cached", file,
+                    NULL);
        if (run_command(&child))
                die(_("Could not apply '%s'"), file);
 

diff --git a/builtin/fsck.c b/builtin/fsck.c
index 27b9e78094d9816141e81dff359393b81b458d82..9e54892311d5a698d51abb66ac73892c739732a1 100644 (file)
--- a/builtin/fsck.c
+++ b/builtin/fsck.c
@@ -944,15 +944,13 @@ int cmd_fsck(int argc, const char **argv, const char *prefix)
 
        if (the_repository->settings.core_commit_graph) {
                struct child_process commit_graph_verify = CHILD_PROCESS_INIT;
-               const char *verify_argv[] = { "commit-graph", "verify", NULL, NULL, NULL };
 
                prepare_alt_odb(the_repository);
                for (odb = the_repository->objects->odb; odb; odb = odb->next) {
                        child_process_init(&commit_graph_verify);
-                       commit_graph_verify.argv = verify_argv;
                        commit_graph_verify.git_cmd = 1;
-                       verify_argv[2] = "--object-dir";
-                       verify_argv[3] = odb->path;
+                       strvec_pushl(&commit_graph_verify.args, "commit-graph",
+                                    "verify", "--object-dir", odb->path, NULL);
                        if (run_command(&commit_graph_verify))
                                errors_found |= ERROR_COMMIT_GRAPH;
                }
@@ -960,15 +958,13 @@ int cmd_fsck(int argc, const char **argv, const char *prefix)
 
        if (the_repository->settings.core_multi_pack_index) {
                struct child_process midx_verify = CHILD_PROCESS_INIT;
-               const char *midx_argv[] = { "multi-pack-index", "verify", NULL, NULL, NULL };
 
                prepare_alt_odb(the_repository);
                for (odb = the_repository->objects->odb; odb; odb = odb->next) {
                        child_process_init(&midx_verify);
-                       midx_verify.argv = midx_argv;
                        midx_verify.git_cmd = 1;
-                       midx_argv[2] = "--object-dir";
-                       midx_argv[3] = odb->path;
+                       strvec_pushl(&midx_verify.args, "multi-pack-index",
+                                    "verify", "--object-dir", odb->path, NULL);
                        if (run_command(&midx_verify))
                                errors_found |= ERROR_MULTI_PACK_INDEX;
                }

diff --git a/builtin/help.c b/builtin/help.c
index 75cd2fb407f6ebd3ce24b8e3588cf384d7b48699..d387131dd836c4eed62aa243ddbc1789934db8e0 100644 (file)
--- a/builtin/help.c
+++ b/builtin/help.c
@@ -212,11 +212,10 @@ static int check_emacsclient_version(void)
 {
        struct strbuf buffer = STRBUF_INIT;
        struct child_process ec_process = CHILD_PROCESS_INIT;
-       const char *argv_ec[] = { "emacsclient", "--version", NULL };
        int version;
 
        /* emacsclient prints its version number on stderr */
-       ec_process.argv = argv_ec;
+       strvec_pushl(&ec_process.args, "emacsclient", "--version", NULL);
        ec_process.err = -1;
        ec_process.stdout_to_stderr = 1;
        if (start_command(&ec_process))

diff --git a/builtin/merge.c b/builtin/merge.c
index ea3112e0c0b3acc9f4439430fb19e596a9fc9093..5f0476b0b761b60fdddffbc15217bd4e345570e4 100644 (file)
--- a/builtin/merge.c
+++ b/builtin/merge.c
@@ -310,10 +310,9 @@ static int save_state(struct object_id *stash)
        int len;
        struct child_process cp = CHILD_PROCESS_INIT;
        struct strbuf buffer = STRBUF_INIT;
-       const char *argv[] = {"stash", "create", NULL};
        int rc = -1;
 
-       cp.argv = argv;
+       strvec_pushl(&cp.args, "stash", "create", NULL);
        cp.out = -1;
        cp.git_cmd = 1;
 

diff --git a/builtin/notes.c b/builtin/notes.c
index 71c59583a17f8da6eb948248e19a262781091f28..85d1abad884242695da7e5e841c3341c985ab5d2 100644 (file)
--- a/builtin/notes.c
+++ b/builtin/notes.c
@@ -134,14 +134,13 @@ static void copy_obj_to_fd(int fd, const struct object_id *oid)
 
 static void write_commented_object(int fd, const struct object_id *object)
 {
-       const char *show_args[5] =
-               {"show", "--stat", "--no-notes", oid_to_hex(object), NULL};
        struct child_process show = CHILD_PROCESS_INIT;
        struct strbuf buf = STRBUF_INIT;
        struct strbuf cbuf = STRBUF_INIT;
 
        /* Invoke "git show --stat --no-notes $object" */
-       show.argv = show_args;
+       strvec_pushl(&show.args, "show", "--stat", "--no-notes",
+                    oid_to_hex(object), NULL);
        show.no_stdin = 1;
        show.out = -1;
        show.err = 0;

diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c
index 49b846d960522ad1a5f29f7394bca4fa24e5b622..6149d50796584e1b7d33fe6023fd5e86297f3d7c 100644 (file)
--- a/builtin/receive-pack.c
+++ b/builtin/receive-pack.c
@@ -1370,22 +1370,10 @@ static const char *push_to_deploy(unsigned char *sha1,
                                  struct strvec *env,
                                  const char *work_tree)
 {
-       const char *update_refresh[] = {
-               "update-index", "-q", "--ignore-submodules", "--refresh", NULL
-       };
-       const char *diff_files[] = {
-               "diff-files", "--quiet", "--ignore-submodules", "--", NULL
-       };
-       const char *diff_index[] = {
-               "diff-index", "--quiet", "--cached", "--ignore-submodules",
-               NULL, "--", NULL
-       };
-       const char *read_tree[] = {
-               "read-tree", "-u", "-m", NULL, NULL
-       };
        struct child_process child = CHILD_PROCESS_INIT;
 
-       child.argv = update_refresh;
+       strvec_pushl(&child.args, "update-index", "-q", "--ignore-submodules",
+                    "--refresh", NULL);
        child.env = env->v;
        child.dir = work_tree;
        child.no_stdin = 1;
@@ -1396,7 +1384,8 @@ static const char *push_to_deploy(unsigned char *sha1,
 
        /* run_command() does not clean up completely; reinitialize */
        child_process_init(&child);
-       child.argv = diff_files;
+       strvec_pushl(&child.args, "diff-files", "--quiet",
+                    "--ignore-submodules", "--", NULL);
        child.env = env->v;
        child.dir = work_tree;
        child.no_stdin = 1;
@@ -1405,11 +1394,12 @@ static const char *push_to_deploy(unsigned char *sha1,
        if (run_command(&child))
                return "Working directory has unstaged changes";
 
-       /* diff-index with either HEAD or an empty tree */
-       diff_index[4] = head_has_history() ? "HEAD" : empty_tree_oid_hex();
-
        child_process_init(&child);
-       child.argv = diff_index;
+       strvec_pushl(&child.args, "diff-index", "--quiet", "--cached",
+                    "--ignore-submodules",
+                    /* diff-index with either HEAD or an empty tree */
+                    head_has_history() ? "HEAD" : empty_tree_oid_hex(),
+                    "--", NULL);
        child.env = env->v;
        child.no_stdin = 1;
        child.no_stdout = 1;
@@ -1418,9 +1408,9 @@ static const char *push_to_deploy(unsigned char *sha1,
        if (run_command(&child))
                return "Working directory has staged changes";
 
-       read_tree[3] = hash_to_hex(sha1);
        child_process_init(&child);
-       child.argv = read_tree;
+       strvec_pushl(&child.args, "read-tree", "-u", "-m", hash_to_hex(sha1),
+                    NULL);
        child.env = env->v;
        child.dir = work_tree;
        child.no_stdin = 1;
@@ -2575,16 +2565,14 @@ int cmd_receive_pack(int argc, const char **argv, const char *prefix)
                run_update_post_hook(commands);
                string_list_clear(&push_options, 0);
                if (auto_gc) {
-                       const char *argv_gc_auto[] = {
-                               "gc", "--auto", "--quiet", NULL,
-                       };
                        struct child_process proc = CHILD_PROCESS_INIT;
 
                        proc.no_stdin = 1;
                        proc.stdout_to_stderr = 1;
                        proc.err = use_sideband ? -1 : 0;
                        proc.git_cmd = proc.close_object_store = 1;
-                       proc.argv = argv_gc_auto;
+                       strvec_pushl(&proc.args, "gc", "--auto", "--quiet",
+                                    NULL);
 
                        if (!start_command(&proc)) {
                                if (use_sideband)

diff --git a/builtin/replace.c b/builtin/replace.c
index 946938d011ee8ea69040b931c98acd270bbcd46d..6ff1734d5879d36c994d0ab4b82b95e60a348750 100644 (file)
--- a/builtin/replace.c
+++ b/builtin/replace.c
@@ -258,11 +258,10 @@ static int import_object(struct object_id *oid, enum object_type type,
                return error_errno(_("unable to open %s for reading"), filename);
 
        if (!raw && type == OBJ_TREE) {
-               const char *argv[] = { "mktree", NULL };
                struct child_process cmd = CHILD_PROCESS_INIT;
                struct strbuf result = STRBUF_INIT;
 
-               cmd.argv = argv;
+               strvec_push(&cmd.args, "mktree");
                cmd.git_cmd = 1;
                cmd.in = fd;
                cmd.out = -1;

diff --git a/editor.c b/editor.c
index fdd3eeafa94791791aefc976988f4cd0b5a5f58f..d92a8d9ab5bb4f48d2eff713464f1da744ce50b1 100644 (file)
--- a/editor.c
+++ b/editor.c
@@ -55,7 +55,6 @@ static int launch_specified_editor(const char *editor, const char *path,
 
        if (strcmp(editor, ":")) {
                struct strbuf realpath = STRBUF_INIT;
-               const char *args[] = { editor, NULL, NULL };
                struct child_process p = CHILD_PROCESS_INIT;
                int ret, sig;
                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,
                }
 
                strbuf_realpath(&realpath, path, 1);
-               args[1] = realpath.buf;
 
-               p.argv = args;
+               strvec_pushl(&p.args, editor, realpath.buf, NULL);
                p.env = env;
                p.use_shell = 1;
                p.trace2_child_class = "editor";

diff --git a/sequencer.c b/sequencer.c
index ea96837cde3bad6faa9cf016323fe3267a538cb4..6e02210db7a50b84a078c6ce02d6d3b8288f423d 100644 (file)
--- a/sequencer.c
+++ b/sequencer.c
@@ -1164,18 +1164,14 @@ static int run_rewrite_hook(const struct object_id *oldoid,
                            const struct object_id *newoid)
 {
        struct child_process proc = CHILD_PROCESS_INIT;
-       const char *argv[3];
        int code;
        struct strbuf sb = STRBUF_INIT;
+       const char *hook_path = find_hook("post-rewrite");
 
-       argv[0] = find_hook("post-rewrite");
-       if (!argv[0])
+       if (!hook_path)
                return 0;
 
-       argv[1] = "amend";
-       argv[2] = NULL;
-
-       proc.argv = argv;
+       strvec_pushl(&proc.args, hook_path, "amend", NULL);
        proc.in = -1;
        proc.stdout_to_stderr = 1;
        proc.trace2_hook_name = "post-rewrite";

diff --git a/upload-pack.c b/upload-pack.c
index c78d55bc674ed2b22b7f4417173242ad4e53bfd0..9b5db32623fa3100a697a6ddf64cccc75865563f 100644 (file)
--- a/upload-pack.c
+++ b/upload-pack.c
@@ -596,14 +596,11 @@ static int do_reachable_revlist(struct child_process *cmd,
                                struct object_array *reachable,
                                enum allow_uor allow_uor)
 {
-       static const char *argv[] = {
-               "rev-list", "--stdin", NULL,
-       };
        struct object *o;
        FILE *cmd_in = NULL;
        int i;
 
-       cmd->argv = argv;
+       strvec_pushl(&cmd->args, "rev-list", "--stdin", NULL);
        cmd->git_cmd = 1;
        cmd->no_stderr = 1;
        cmd->in = -1;