Re: [PATCH] run_command: teach API users to use embedded 'args' more

From: Jeff King <peff@peff.net>
Date: Aug 27, 2020, 04:21 UTC
Message-ID: <20200827042157.GC3346457@coredump.intra.peff.net>
In-Reply-To: <xmqqmu2ht58g.fsf_-_@gitster.c.googlers.com>

On Wed, Aug 26, 2020 at 03:25:03PM -0700, Junio C Hamano wrote:

Show 6 quoted lines

> The child_process structure has an embedded strvec for formulating
> the command line argument list these days, but code that predates
> the wide use of it prepared a separate char *argv[] array and
> manually set the child_process.argv pointer point at it.
> 
> Teach these old-style code to lose the separate argv[] array.

I think the result is much nicer and less error-prone (especially the ones that pre-size the array with NULLs and fill in the components later). It incurs a few extra mallocs at run-time, but on top of an execve(), that's a drop in the bucket.

I've actually considered dropping child_process.argv entirely. Having two separate ways to do the same thing gives the potential for confusion. But I never dug into whether any existing callers would be made worse for it (I kind of doubt it, though; worst case they can use strvec_pushv). There are still several left after this patch, it seems.

Likewise for child_process.env_array.

-Peff