Re: [PATCH] worktree: fix leak in check_clean_worktree()

Eric Sunshine [off-list ref] writes:

Agreed. The current usage in worktree.c is a bit too familiar with the
current internal implementation of run_command(). Reinitializing the
child_process struct or using a separate one would be a good cleanup.

quoted

-- >8 --
Subject: [PATCH] worktree: fix leak in check_clean_worktree()

We allocate a child_env strvec but never free its memory. Instead, let's
just use the strvec that our child_process struct provides, which is
cleaned up automatically when we run the command.

And while we're moving the initialization of the child_process around,
let's switch it to use the official init function (zero-initializing it
works OK, since strvec is happy enough with that, but it sets a bad
example).

The various memset()'s in worktree.c seem to have been inherited (and
multiplied) from Duy's original "git checkout --to" implementation
(which later became the basis for "git worktree add" after which it
mutated significantly), and "git checkout --to" predates introduction
of child_process_init().

quoted

diff --git a/builtin/worktree.c b/builtin/worktree.c

@@ -924,7 +924,6 @@ static int move_worktree(int ac, const char **av, const char *prefix)
-       struct strvec child_env = STRVEC_INIT;

@@ -935,15 +934,14 @@ static void check_clean_worktree(struct worktree *wt,
-       strvec_pushf(&child_env, "%s=%s/.git",
+       child_process_init(&cp);
+       strvec_pushf(&cp.env_array, "%s=%s/.git",
                     GIT_DIR_ENVIRONMENT, wt->path);
-       strvec_pushf(&child_env, "%s=%s",
+       strvec_pushf(&cp.env_array, "%s=%s",
                     GIT_WORK_TREE_ENVIRONMENT, wt->path);
-       memset(&cp, 0, sizeof(cp));
-       cp.env = child_env.v;

Looks good to me. For what it's worth:

Reviewed-by: Eric Sunshine <redacted>

Thanks, both.  This looks good.