On Thu, Aug 27, 2020 at 12:44 AM Jeff King <peff@xxxxxxxx> 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). Right. I wonder why the author of 7f44e3d1de (worktree: make setup of new HEAD distinct from worktree population, 2015-07-17) chose to clear cp.args manually like that. > - check_clean_worktree() only uses it once, and could drop the > separate child_env (and in fact appears to leak it) Perhaps this unnecessary 'child_env' strvec was a copy/paste from add_worktree()? But certainly cp.env_array would be simpler and avoid the leak.
