Re: [PATCH] revision: replace "struct cmdline_pathspec" with argv_array
- From
- Martin Ågren <martin.agren@gmail.com>
- Date
- Sep 21, 2017, 03:57 UTC
- Message-ID
- <CAN0heSoWnUXSjP1HtRTKpBiOc9JNzZN_+m1O3_Wn+rE43MpPhg@mail.gmail.com>
- In-Reply-To
- <20170920203659.xqy76bg5nfabvbfx@sigill.intra.peff.net>
On 20 September 2017 at 22:36, Jeff King <peff@peff.net> wrote:
Show 15 quoted lines
> 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