Re: [PATCH] run_command: teach API users to use embedded 'args' more

From

Eric Sunshine <sunshine@sunshineco.com>

Date

Aug 27, 2020, 05:03 UTC

Message-ID

<CAPig+cSA56xgNN0WP4t+YoyNU8fGf5eaz__=4Vh+s=He-tG=DA@mail.gmail.com>

In-Reply-To

<20200827044420.GA3360616@coredump.intra.peff.net>

On Thu, Aug 27, 2020 at 12:44 AM Jeff King <peff@peff.net> wrote:

> On Thu, Aug 27, 2020 at 12:31:52AM -0400, Eric Sunshine wrote:
> > Nevertheless, that isn't much of a reason to keep child_process.env.
> > Refactoring add_worktree() a bit to rebuild the environment strvec
> > on-demand wouldn't be very difficult.
>
> I think they'd still be one-liner changes, like:
>
> -       cp.env = child_env.v;
> +       strvec_pushv(&cp.env_array, child_env.v);

Nice and simple.

> I think there are other opportunities for cleanup, too:

True on both counts.

>   - the code right above the second hunk clears cp.args manually. That
>     shouldn't be necessary because run_command() will leave it in a
>     blank state (and we're already relying on that, since otherwise we'd
>     be left with cruft in other fields from the previous run).

>   - check_clean_worktree() only uses it once, and could drop the
>     separate child_env (and in fact appears to leak it)