Re: [PATCH v2 1/9] worktree: remove redundant NULL-ing of "cp.argv

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

Eric Sunshine <sunshine@sunshineco.com> writes:

> ... At best, following this change,
> git-worktree is only working "by accident" if the underlying
> child_process::args.v doesn't get reallocated between run_command()
> invocations. Relying upon this "by accident" behavior feels rather
> unsafe.

Very true.  Relying on the "if argv is null, point it at args.v"
assignment at the very beginning of the start_command() function is
safe because by that time the reallocations have happened already
if needed.

The pattern with or without NULLing is

	initialize cp
	push to cp.args
	use cp

	/* cp.argv = NULL */
	strvec_clear(&cp.args);
	push to cp.args

and strvec_clear() frees the underying array, and the first push
will reallocates from NULL, so there is no guarantee that cp.argv
in the first use that used to be pointing at cp.args that has
already been freed is still valid.

Thanks for spotting this.  Has this patch ever been tested with
sanitizer?  Do we have gap in test coverage?