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

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

[René Scharfe]

The push specs are kept in a strvec, whose array is NULL-terminated.
Pass only that to the protocol handlers, which avoids dealing with item
counts and their conversions from size_t to int, slightly simplifying
the code.

Signed-off-by: René Scharfe <l.s.r@web.de>
---
 remote-curl.c | 20 +++++++++-----------
 1 file changed, 9 insertions(+), 11 deletions(-)

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");
@@ -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);
 
 	if (run_command(&child))
 		die(_("git-http-push failed"));
 	return 0;
 }
 
-static int push_git(struct discovery *heads, int nr_spec, const char **specs)
+static int push_git(struct discovery *heads, const char **specs)
 {
 	struct rpc_state rpc = RPC_STATE_INIT;
 	int i, err;
@@ -1400,8 +1398,8 @@ static int push_git(struct discovery *heads, int nr_spec, const char **specs)
 		strvec_push(&args, "--force-if-includes");
 
 	strvec_push(&args, "--stdin");
-	for (i = 0; i < nr_spec; i++)
-		packet_buf_write(&preamble, "%s\n", specs[i]);
+	for (; *specs; specs++)
+		packet_buf_write(&preamble, "%s\n", *specs);
 	packet_buf_flush(&preamble);
 
 	memset(&rpc, 0, sizeof(rpc));
@@ -1416,15 +1414,15 @@ static int push_git(struct discovery *heads, int nr_spec, const char **specs)
 	return err;
 }
 
-static int push(int nr_spec, const char **specs)
+static int push(const char **specs)
 {
 	struct discovery *heads = discover_refs("git-receive-pack", 1);
 	int ret;
 
 	if (heads->proto_git)
-		ret = push_git(heads, nr_spec, specs);
+		ret = push_git(heads, specs);
 	else
-		ret = push_dav(nr_spec, specs);
+		ret = push_dav(specs);
 	free_discovery(heads);
 	return ret;
 }
@@ -1448,7 +1446,7 @@ static void parse_push(struct strbuf *buf)
 			break;
 	} while (1);
 
-	ret = push(specs.nr, specs.v);
+	ret = push(specs.v);
 	printf("\n");
 	fflush(stdout);
 
-- 
2.55.0

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

[Patrick Steinhardt]

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.

> @@ -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.

Other than those nits the patch looks good to me, thanks!

Patrick

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

[René Scharfe]

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é