Re: [PATCH] remote-curl: simplify passing of push specs

["René Scharfe" <l.s.r@web.de>]

On 7/15/26 8:41 AM, Patrick Steinhardt wrote:
> On Wed, Jul 15, 2026 at 06:41:17AM +0200, René Scharfe wrote:
>> diff --git a/remote-curl.c b/remote-curl.c
>> index 9e614c5567..2c35dd5240 100644
>> --- a/remote-curl.c
>> +++ b/remote-curl.c
>> @@ -1340,10 +1340,9 @@ static void parse_get(const char *arg)
>>  	fflush(stdout);
>>  }
>>  
>> -static int push_dav(int nr_spec, const char **specs)
>> +static int push_dav(const char **specs)
>>  {
>>  	struct child_process child = CHILD_PROCESS_INIT;
>> -	size_t i;
>>  
>>  	child.git_cmd = 1;
>>  	strvec_push(&child.args, "http-push");
> 
> I wonder whether the interface would be even better if we simply passed
> around a `const struct strvec *` directly. That makes it explicit what
> kind of guarantees we have, and all transitive callers already have one
> available anyway.

You mean that passing a managed array instead of a plain NULL-terminated
one would make more places visibly safer at almost no cost?

>> @@ -1353,15 +1352,14 @@ static int push_dav(int nr_spec, const char **specs)
>>  	if (options.verbosity > 1)
>>  		strvec_push(&child.args, "--verbose");
>>  	strvec_push(&child.args, url.buf);
>> -	for (i = 0; i < nr_spec; i++)
>> -		strvec_push(&child.args, specs[i]);
>> +	strvec_pushv(&child.args, specs);
> 
> I thought that we had something like `strvec_pushvec()` that knew to
> also optimize for this case so that we don't have to reallocate the
> vector multiple times. And if we had that function it would even be more
> efficient to pass it down the stack. But we seemingly don't have it, so
> that argument is kind of moot.
We could add one.  Not sure it would make a measurable difference; if
the number of specs is huge there are probably other costs that dwarf
pushing them to a strvec.

I have to admit that the simplicity of strvec_pushv() nudged me towards
using a NULL-terminated array here, though.  So just having a
strvec_pushvec() available could guide towards using the length-limited
strvec instead of a simpler NULL-terminated array (which explodes if
left unterminated).

René