On Thu, Jan 31, 2019 at 06:59:17PM +0100, Thomas Braun wrote:
> > Junio C Hamano <gitster@xxxxxxxxx> hat am 29. Januar 2019 um 23:15 geschrieben:
>
> [...]
>
> > * mk/use-size-t-in-zlib (2018-10-15) 1 commit
> > - zlib.c: use size_t for size
> >
> > The wrapper to call into zlib followed our long tradition to use
> > "unsigned long" for sizes of regions in memory, which have been
> > updated to use "size_t".
> >
>
> I've started playing around with the patch from Thorsten [1] for getting unsigned long replaced in more places so that you can commit large files on platforms like Windows there unsigned long is 32-bit even on 64-bit OSes.
>
> And the first thing which bugs out when I do a quick test with committing a large file and fsck the repo is in zlib.c:
>
> if (s->z.total_out != s->total_out + bytes_produced)
> BUG("total_out mismatch");
>
> here s->z.total_out is an unsigned long and s->total_out is size_t and this triggers the BUG message once the unsigned long wraps. There is even an FAQ entry for zlib at [2] which warns about that potential issue.
>
> So I would think that something like
>
> ----------->8
>
> diff --git a/zlib.c b/zlib.c
> index 197a1acc7b..9cc6421eba 100644
> --- a/zlib.c
> +++ b/zlib.c
> @@ -51,13 +51,9 @@ static void zlib_post_call(git_zstream *s)
>
> bytes_consumed = s->z.next_in - s->next_in;
> bytes_produced = s->z.next_out - s->next_out;
> - if (s->z.total_out != s->total_out + bytes_produced)
> - BUG("total_out mismatch");
> - if (s->z.total_in != s->total_in + bytes_consumed)
> - BUG("total_in mismatch");
>
> - s->total_out = s->z.total_out;
> - s->total_in = s->z.total_in;
> + s->total_out += bytes_produced;
> + s->total_in += bytes_consumed;
> s->next_in = s->z.next_in;
> s->next_out = s->z.next_out;
> s->avail_in -= bytes_consumed;
>
> -----------8<
>
> would make the patch [3] more complete IMHO.
>
> Another potential issue in that patch is that the signature change in git_deflate_bound forces size to unsigned long on the call to deflateBound (for newer zlib versions) and if that conversion is not faithful this will certainly not work.
>
> Just my 2cents I'm not vetoing anything here,
> Thomas
Thanks for digging.
Do you think that you can provide a new version of the patch ?
Or, may be better, a patch on top of that ?
>
> [1]: http://public-inbox.org/git/20181120050456.16715-1-tboegi@xxxxxx/
> [2]: http://www.zlib.net/zlib_faq.html#faq32
> [3]: http://public-inbox.org/git/20181012204229.11890-1-tboegi@xxxxxx/
