Re: [PATCH 12/12] git-zlib: widen `git_deflate_bound()` to `size_t`

[Johannes Schindelin <Johannes.Schindelin@gmx.de>]

Hi Patrick,

On Wed, 5 Aug 2026, Patrick Steinhardt wrote:

> On Thu, Jul 09, 2026 at 04:49:39PM +0000, Johannes Schindelin via GitGitGadget wrote:
> > From: Johannes Schindelin <johannes.schindelin@gmx.de>
> > 
> > All four `unsigned long`/`int`/`ssize_t` receivers across archive-zip,
> > diff, http-push and t/helper/test-pack-deltas were widened to `size_t`
> > in the prior commits, and remote-curl and fast-import were already
> > there. With every caller prepared, both the parameter and the return
> > type can now move without introducing any silent narrowing.
> 
> Nit, feel free to ignore: I feel like all of these patches could've been
> squashed into a single one, as they're trivial enough.

I like them trivial and small ;-)

> > For inputs above zlib's `uLong` range (i.e. >4 GiB on platforms where
> > `uLong` is 32-bit, notably 64-bit Windows), defer to zlib's stored-block
> > formula (the same fallback it would itself use for an unknown stream
> > state) plus the worst-case wrapper overhead. The existing path through
> > `deflateBound()` is unchanged for inputs that fit.
> 
> A link or something like that to the formula would've helped here, as
> I'm not familiar with this mechanism.

Right. I added two references to the commit message.

> 
> > diff --git a/git-zlib.c b/git-zlib.c
> > index d21adb3bf5..ebbbcc6d1a 100644
> > --- a/git-zlib.c
> > +++ b/git-zlib.c
> > @@ -167,9 +167,21 @@ int git_inflate(git_zstream *strm, int flush)
> >  	return status;
> >  }
> >  
> > -unsigned long git_deflate_bound(git_zstream *strm, unsigned long size)
> > +size_t git_deflate_bound(git_zstream *strm, size_t size)
> >  {
> > -	return deflateBound(&strm->z, size);
> > +#if SIZE_MAX > ULONG_MAX
> > +	if (size > maximum_unsigned_value_of_type(uLong))
> > +		/*
> > +		 * deflateBound() takes uLong, which is 32-bit on
> > +		 * Windows. For inputs above that range, return zlib's
> > +		 * stored-block formula (the conservative path it would
> > +		 * itself use for an unknown stream state) plus the
> > +		 * worst-case wrapper overhead.
> > +		 */
> > +		return size + (size >> 5) + (size >> 7) + (size >> 11)
> > +			+ 7 + 18;
> > +#endif
> 
> So is the idea here that we estimate the highest number of bytes that
> the deflated size could end up with?

Precisely. And the formula in zlib is a bit complex, it calculates a
"fixedlen" and a "storelen" for two different ways to represent the worst
case size. But for large values, only `storelen` matters, therefore we can
get away with a much simpler logic here.

FWIW zlib v1.3.2 added `deflateBound_z()`, which accepts `size_t` (or more
precisely: `z_size_t`). However, v1.3.2 is only 7 months old, so I'll be
retired by the time Debian stable gets it :-P

Ciao,
Johannes

> 
> Patrick
>