Re: [PATCH 12/12] git-zlib: widen `git_deflate_bound()` to `size_t`
Johannes Schindelin <[email protected]> Wed, 5 Aug 2026 15:58:43 +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:39PM +0000, Johannes Schindelin via GitGitGadget wrote: > > From: Johannes Schindelin <[email protected]> > > > > 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 >