On Tue, Oct 11, 2022 at 02:43:24PM -0700, Junio C Hamano wrote:
> > - if (run_command_v_opt(argv.v, RUN_GIT_CMD)) {
> > + if (run_command_opt_l(RUN_GIT_CMD, "-C", repo, "sparse-checkout",
> > + "set", NULL)) {
>
> And this does give us protection from the "Programmers can give
> unterminated list to run_command_v_opt() by mistake", which is not
> really solved mechanically even if the list is prepared with the
> strvec API (because the compiler has to be smart enough to know that
> argv.v was prepared with proper use of the API), which is nice.
Yeah, I agree this addresses the point I raised (which I am somewhat
regretting raising, as IMHO it was not worth the amount of discussion
that has ensued).
Since nobody asked, my _real_ opinion is that I prefer René's original
that used an actual struct, and its auto-freeing strvec. And I'd be
happy to see all of the run_command_v() variants go away entirely. It
does save a few lines, but with modern niceties, it's not very many, and
it's much less flexible.
I resisted saying so earlier because I do not think it is even worth
anybody's time to think about, let alone implement.
-Peff
