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

[Junio C Hamano <gitster@pobox.com>]

Eric Sunshine <sunshine@sunshineco.com> 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.
>
>> -- >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().
>
>> 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 <sunshine@sunshineco.com>

Thanks, both.  This looks good.