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 >