Jeff King [off-list ref] 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.