Re: [PATCH 1/2] difftool: use "struct strvec" API in run_{dir,file}_diff()

From: Jeff King <peff@peff.net>
Date: Sep 12, 2021, 22:41 UTC
Message-ID: <YT6CMdITxaZZqufr@coredump.intra.peff.net>
In-Reply-To: <YT6BnnXeAWn8BycF@coredump.intra.peff.net>

On Sun, Sep 12, 2021 at 06:39:26PM -0400, Jeff King wrote:

Show 9 quoted lines

> It only becomes apparent with the second patch. I would have found it
> much easier to understand with something like the patch below. And then
> a further patch to use strvec_pushv instead of manually looping (even
> getting rid of the argc parameters entirely!), and one to convert
> run_file_diff() to use a struct child_process (which fixes its memory
> leak).
> 
> -- >8 --
> difftool: prepare "diff" cmdline in cmd_difftool()

Note that this actually introduces a new leak of the strvec in the caller. So it would probably want the "ret =" think I suggested to be squashed in.

(I wasn't really planning to make a finished patch, but just trying to illustrate what had confused me in your original. I ended up closer than I had planned, though).

-Peff