On 20 September 2017 at 22:36, Jeff King <peff@xxxxxxxx> wrote: > On Wed, Sep 20, 2017 at 04:25:52PM -0400, Jeff King wrote: > >> Isn't this whole thing just an argv_array, and this is argv_array_pushv? >> We even NULL-terminate it manually later on! >> >> So rather than increasing the line count by adding >> free_cmdline_pathspec, I think we could actually _reduce_ it by >> converting to an argv array, as below. And then adding in your free >> would be one extra line. > > Here it is with a commit message, and that final free added. > > Sorry for stealing your patch, but I didn't want to suggest "couldn't > you replace this with argv_array" without actually seeing if it was > possible. At which point the patch was pretty much done. No need to be sorry. I wasn't aware of the argv_array thing, thanks for seeing the bigger picture. I haven't looked into the details, or your and Jonathan's discussion, but just from a cursory look this looks way better. It reuses existing infrastructure, and then this: > revision.c | 39 +++++++++++---------------------------- > 1 file changed, 11 insertions(+), 28 deletions(-) Martin