All of lore.kernel.org
 help / color / mirror / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: "Johannes Schindelin via GitGitGadget" <gitgitgadget@gmail.com>
Cc: git@vger.kernel.org,  Johannes Schindelin <johannes.schindelin@gmx.de>
Subject: Re: [PATCH] zlib: properly clamp to uLong
Date: Thu, 18 Jun 2026 10:03:06 -0700	[thread overview]
Message-ID: <xmqqzf0rrdbp.fsf@gitster.g> (raw)
In-Reply-To: <pull.2153.git.1781790619424.gitgitgadget@gmail.com> (Johannes Schindelin via GitGitGadget's message of "Thu, 18 Jun 2026 13:50:18 +0000")

The original in js/objects-larger-than-4gb-on-windows says things
like:

+	s->z.total_in = (uLong)(s->total_in & ULONG_MAX_VALUE);
+	s->z.total_out = (uLong)(s->total_out & ULONG_MAX_VALUE);

Your patch ...

> +static inline uLong zlib_uLong_cap(size_t s)
> +{
> +	return s < ULONG_MAX_VALUE ? (uLong)s : ULONG_MAX_VALUE;
> +}
> +
>  static void zlib_pre_call(git_zstream *s)
>  {
>  	s->z.next_in = s->next_in;
>  	s->z.next_out = s->next_out;
> -	s->z.total_in = (uLong)(s->total_in & ULONG_MAX_VALUE);
> -	s->z.total_out = (uLong)(s->total_out & ULONG_MAX_VALUE);
> +	s->z.total_in = zlib_uLong_cap(s->total_in);
> +	s->z.total_out = zlib_uLong_cap(s->total_out);

... is an obvious fix for that.

> @@ -60,7 +65,7 @@ static void zlib_post_call(git_zstream *s, int status)
>  	 * We track our own totals and verify only the low bits match.
>  	 */
>  	if ((s->z.total_out & ULONG_MAX_VALUE) !=
> -	    ((s->total_out + bytes_produced) & ULONG_MAX_VALUE))
> +	    ((zlib_uLong_cap(s->total_out) + bytes_produced) & ULONG_MAX_VALUE))
>  		BUG("total_out mismatch");

Because we now clamp (not "taking lower bits of") s->total_out to a
value between 0..4GB and store it in s->z.total_out in pre-call, let
zlib do its thing that increments s->z.total_out modulo 4GB, and we
clamp the s->total_out (before incrementing) the same way in
post_call here, both sides of "!=" above even out.  But the comment
before this comparison that claims that "we ... verify only the low
bits match" is a bit off the reality, I suspect.

> @@ -68,7 +73,7 @@ static void zlib_post_call(git_zstream *s, int status)
>  	 */
>  	if (status != Z_NEED_DICT &&
>  	    (s->z.total_in & ULONG_MAX_VALUE) !=
> -	    ((s->total_in + bytes_consumed) & ULONG_MAX_VALUE))
> +	    ((zlib_uLong_cap(s->total_in) + bytes_consumed) & ULONG_MAX_VALUE))
>  		BUG("total_in mismatch");
>  
>  	s->total_out += bytes_produced;
>
> base-commit: 7a094d68a27e321a99c8ab6b700909e503904bd9

      reply	other threads:[~2026-06-18 17:03 UTC|newest]

Thread overview: 2+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-06-18 13:50 [PATCH] zlib: properly clamp to uLong Johannes Schindelin via GitGitGadget
2026-06-18 17:03 ` Junio C Hamano [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=xmqqzf0rrdbp.fsf@gitster.g \
    --to=gitster@pobox.com \
    --cc=git@vger.kernel.org \
    --cc=gitgitgadget@gmail.com \
    --cc=johannes.schindelin@gmx.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.