On 20 September 2017 at 22:36, Jeff King <p...@peff.net> 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

Reply via email to