Re: [BUG] git stash show -p with invalid option aborts with double-free in show_stash() (strvec_clear)

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

Jeff King <peff@peff.net> writes:

> I think we'll have leaked the string holding "-p" in this instance,
> though. We probably need to pass in a setup_revision_opt struct with its
> free_removed_argv_elements flag set.
>
> That's true even without your patch, too, of course.

Yeah, while I was preparing the "alternative", I noticed that option
being paid attention to by the code, but you are right.  Anybody who
passes strvec (which owns its contents, unlike the traditional "we
got this argv[] from the operating system" callers) needs to flip
that bit set, or they would leak.

> I'm mildly
> surprised that the test suite doesn't hit this in leak-checking mode,
> since it is a problem any time we rearrange argv. E.g., I think:
>
>   git stash show -p --
>
> leaks (I was surprised that "stash show -p --stat" didn't leak, but it
> doesn't seem to rearrange?).

Yeah.

> I wonder if the best solution is a setup_revisions() wrapper for strvecs
> that will:
>
>   - turn on the free_removed_argv_elements option automatically
>
>   - collect the return value of setup_revisions() and use it to fix
>     the .nr field of the strvec
>
>   - restore the NULL invariant at the end of the array (though I would
>     also be happy if setup_revisions() just did this itself)

That would be nice.  I only did the third one in my "alternative"
patch I sent earlier.