Eric Sunshine [off-list ref] 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?