[PATCH 6/6] revision: retain argv NULL invariant in setup_revisions()

In an argc/argv pair, the entry for argv[argc] is generally NULL. You can iterate by counting up to argc, or by looking for the NULL entry in argv.

When we pass such a pair to setup_revisions(), it shrinks argc to account for the options we consumed and returns the result to the caller. But it doesn't touch the entries after the reduced argc. So argv[argc] will be left pointing at some arbitrary entry rather than NULL.

This isn't the source of any known bugs, since all callers are aware of the limitation and act accordingly. But it's a possible gotcha that may be easy to miss.

Let's set the new argv[argc] to NULL, taking care to free it if the caller asked us to do so.

It is tempting to do likewise for all of the entries afterwards, too, as some of them may also need to be freed (e.g., if coming from a strvec). But doing so isn't entirely trivial, as we munge argc in the function (e.g., when we find "--" and move all of the entries after it into the prune_data list). It would be possible with some light refactoring, but it's probably not worth it. Nobody should ever look at them (they are beyond the revised argc and past the NULL argv entry) outside of strvec cleanup, and setup_revisions_from_strvec() already handles this case.

There's one other interesting gotcha: many callers which do not want to provide arguments just pass 0/NULL for argc/argv. We need to check for this case before assigning the final NULL.

Signed-off-by: Jeff King <peff@peff.net>
---
 revision.c | 6 ++++++
 1 file changed, 6 insertions(+)