Re: [PATCH v2 3/7] builtin/receive-pack: read unpack limit config lazily

> On Sun, Aug 09, 2026 at 02:01:02PM -0500, Justin Tobler wrote:
> > diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c
> > index 78d2911c00..5264d70467 100644
> > --- a/builtin/receive-pack.c
> > +++ b/builtin/receive-pack.c
> > @@ -2333,6 +2320,30 @@ static void push_header_arg(struct strvec *args, struct pack_header *hdr)
> >  		     ntohl(hdr->hdr_version), ntohl(hdr->hdr_entries));
> >  }
> >  
> > +static int get_unpack_limit(struct repository *repo)
> 
> Shouldn't the function return `unsigned int`? We always expect it to be
> a positiv value, and in the final commit we have to add a cast because
> of that.

> > +{
> > +	static int limit = -1;
> 
> Is it really necessary to have this be a static variable? As far as I
> can see we'd only call `unpack()` once. Also, the cache would become
> stale if we ever tried to read the limit for multiple different repos.

> > +	if (limit < 0) {
> > +		int receive_limit = -1;
> > +		int transfer_limit = -1;
> > +
> > +		repo_config_get_int(repo, "receive.unpacklimit",
> > +				    &receive_limit);
> > +		repo_config_get_int(repo, "transfer.unpacklimit",
> > +				    &transfer_limit);
> > +
> > +		if (receive_limit >= 0)
> > +			limit = receive_limit;
> > +		else if (transfer_limit >= 0)
> > +			limit = transfer_limit;
> > +		else
> > +			limit = 100;
> > +	}
> > +
> > +	return limit;
> > +}
> 
> So how about something like this instead?
> 
> 	static unsigned int get_unpack_limit(struct repository *repo)
> 	{
> 		unsigned int limit = 100;
> 		if (!repo_config_get_uint(repo, "receive.unpacklimit", &receive_limit) ||
> 		    !repo_config_get_uint(repo, "receive.unpacklimit", &receive_limit))
> 			/* do nothing */;
> 		return limit;
> 	}