On Sun, Sep 12, 2021 at 06:39:26PM -0400, Jeff King wrote: > 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
