]> git.ipfire.org Git - thirdparty/git.git/commitdiff
worktree: stop being overly intimate with run_command() internals
authorEric Sunshine <sunshine@sunshineco.com>
Thu, 25 Nov 2021 22:52:16 +0000 (23:52 +0100)
committerJunio C Hamano <gitster@pobox.com>
Fri, 26 Nov 2021 06:15:07 +0000 (22:15 -0800)
add_worktree() reuses a `child_process` for three run_command()
invocations, but to do so, it has overly-intimate knowledge of
run-command.c internals. In particular, it knows that it must reset
child_process::argv to NULL for each subsequent invocation[*] in order
for start_command() to latch the newly-populated child_process::args for
each invocation, even though this behavior is not a part of the
documented API. Beyond having overly-intimate knowledge of run-command.c
internals, the reuse of one `child_process` for three run_command()
invocations smells like an unnecessary micro-optimization. Therefore,
stop sharing one `child_process` and instead use a new one for each
run_command() call.

[*] If child_process::argv is not reset to NULL, then subsequent
run_command() invocations will instead incorrectly access a dangling
pointer to freed memory which had been allocated by child_process::args
on the previous run. This is due to the following code in
start_command():

    if (!cmd->argv)
        cmd->argv = cmd->args.v;

Signed-off-by: Eric Sunshine <sunshine@sunshineco.com>
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
builtin/worktree.c

index d22ece93e1a80597e2a14950d7362c37b10c2945..9edd3e2829bb1f9b1148c4c834c4720dd8eb2144 100644 (file)
@@ -355,8 +355,8 @@ static int add_worktree(const char *path, const char *refname,
                goto done;
 
        if (opts->checkout) {
-               cp.argv = NULL;
-               strvec_clear(&cp.args);
+               struct child_process cp = CHILD_PROCESS_INIT;
+               cp.git_cmd = 1;
                strvec_pushl(&cp.args, "reset", "--hard", "--no-recurse-submodules", NULL);
                if (opts->quiet)
                        strvec_push(&cp.args, "--quiet");
@@ -385,12 +385,11 @@ done:
                const char *hook = find_hook("post-checkout");
                if (hook) {
                        const char *env[] = { "GIT_DIR", "GIT_WORK_TREE", NULL };
-                       cp.git_cmd = 0;
+                       struct child_process cp = CHILD_PROCESS_INIT;
                        cp.no_stdin = 1;
                        cp.stdout_to_stderr = 1;
                        cp.dir = path;
                        cp.env = env;
-                       cp.argv = NULL;
                        cp.trace2_hook_name = "post-checkout";
                        strvec_pushl(&cp.args, absolute_path(hook),
                                     oid_to_hex(null_oid()),