Re: [PATCH 03/12] pack-objects: widen delta-cache accounting to `size_t`

Johannes Schindelin <[email protected]> Wed, 5 Aug 2026 15:52:56 +0200 (CEST)
Newsgroups org.kernel.vger.git
Message-ID <[email protected]>
Hi Patrick,

On Wed, 5 Aug 2026, Patrick Steinhardt wrote:

> On Thu, Jul 09, 2026 at 04:49:30PM +0000, Johannes Schindelin via GitGitGadget wrote:
> > diff --git a/builtin/pack-objects.c b/builtin/pack-objects.c
> > index e3760b3492..f89628a760 100644
> > --- a/builtin/pack-objects.c
> > +++ b/builtin/pack-objects.c
> > @@ -260,8 +260,8 @@ static int exclude_promisor_objects_best_effort;
> >  
> >  static int use_delta_islands;
> >  
> > -static unsigned long delta_cache_size = 0;
> > -static unsigned long max_delta_cache_size = DEFAULT_DELTA_CACHE_SIZE;
> > +static size_t delta_cache_size = 0;
> > +static size_t max_delta_cache_size = DEFAULT_DELTA_CACHE_SIZE;
> 
> The only other site that assigns `max_delta_cache_size` does so via
> `git_config_int()`, so we happily accept negative values for
> "pack.deltacachesize". This will cause a change in behaviour here, even
> though arguably the behaviour both before and after this patch is broken
> in the same way.
> 
> Ideally we'd have something like `git_config_size_t()`, or at least use
> `git_config_uint()` here.

Heh, I looked for `git_config_size_t()` and it does not exist, but
`git_config_ssize_t()` exists... Pretty inconsistent. Anyway, I added that
function and use it in the other assignment.

Ciao,
Johannes

> But that could potentially break the case where somebody mistakenly
> configured a negative value and took it as "infinite", which was mostly
> true before.
> 
> In any case, our docs only mention positive values. So maybe this is
> something we could fix while at it.
> 
> Patrick
>