Iffy output given git diff --unified=2147483647

> $ git --versiongit version 2.47.0.windows.2
> 
> $ git diff --unified=2147483647 1.txt 2.txt
> diff --git a/1.txt b/2.txt
> index e53eaa1..1130481 100644
> --- a/1.txt
> +++ b/2.txt
> @@ -1,10 +1,10 @@
>  1
> -2
> +a
>  3
>  4
>  5
>  6
>  7
>  8
>  a
>  0
> @@ -1,10 +1,10 @@
>  1
>  a
>  3
>  4
>  5
>  6
>  7
>  8
> -9
> +a
>  0
> 
> $ git diff --unified=3 1.txt 2.txt
> diff --git a/1.txt b/2.txt
> index e53eaa1..1130481 100644
> --- a/1.txt
> +++ b/2.txt
> @@ -1,10 +1,10 @@
>  1
> -2
> +a
>  3
>  4
>  5
>  6
>  7
>  8
> -9
> +a
>  0
> 
> $ diff --version
> diff (GNU diffutils) 3.10
> Copyright (C) 2023 Free Software Foundation, Inc.
> License GPLv3+: GNU GPL version 3 or later <https://gnu.org/licenses/gpl.html>.
> This is free software: you are free to change and redistribute it.
> There is NO WARRANTY, to the extent permitted by law.
> 
> Written by Paul Eggert, Mike Haertel, David Hayes,
> Richard Stallman, and Len Tower.
> 
> $ diff  --unified=2147483647 1.txt 2.txt
> --- 1.txt       2025-03-12 16:04:06.947099900 +0100
> +++ 2.txt       2025-03-12 16:04:27.131732400 +0100
> @@ -1,10 +1,10 @@
>  1
> -2
> +a
>  3
>  4
>  5
>  6
>  7
>  8
> -9
> +a
>  0

> xdchange_t *xdl_get_hunk(xdchange_t **xscr, xdemitconf_t const *xecfg){
> xdchange_t *xch, *xchp, *lxch;
> long max_common = 2 * xecfg->ctxlen + xecfg->interhunkctxlen;  <- ----
> ...
> }

> xdl_get_hunk() calculates the maximum number of common lines between two
> changes that would fit into the same hunk for the given context options.
> It involves doubling and addition and thus can overflow if the terms are
> huge.
>
> The type of ctxlen and interhunkctxlen in xdemitconf_t is long, while
> the type of the corresponding context and interhunkcontext in struct
> diff_options is int.  On many platforms longs are bigger that ints,
> which prevents the overflow.  On Windows they have the same range and
> the overflow manifests as hunks that are split erroneously and lines
> being repeated between them.
>
> Fix the overflow by checking and not going beyond LONG_MAX.  This allows
> specifying a huge context line count and getting all lines of a changed
> files in a single hunk, as expected.
>
> Reported-by: Jason Cho <jason11choca@proton.me>
> Signed-off-by: René Scharfe <l.s.r@web.de>
> ---
>  t/t4055-diff-context.sh | 10 ++++++++++
>  xdiff/xemit.c           |  8 +++++++-
>  2 files changed, 17 insertions(+), 1 deletion(-)

> diff --git a/xdiff/xemit.c b/xdiff/xemit.c
> index f8e3f25b03..1d40c9cb40 100644
> --- a/xdiff/xemit.c
> +++ b/xdiff/xemit.c
> @@ -43,6 +43,10 @@ static int xdl_emit_record(xdfile_t *xdf, long ri, char const *pre, xdemitcb_t *
>  	return 0;
>  }
>
> +static long saturating_add(long a, long b)
> +{
> +	return signed_add_overflows(a, b) ? LONG_MAX : a + b;
> +}
>
>  /*
>   * Starting at the passed change atom, find the latest change atom to be included
> @@ -52,7 +56,9 @@ static int xdl_emit_record(xdfile_t *xdf, long ri, char const *pre, xdemitcb_t *
>  xdchange_t *xdl_get_hunk(xdchange_t **xscr, xdemitconf_t const *xecfg)
>  {
>  	xdchange_t *xch, *xchp, *lxch;
> -	long max_common = 2 * xecfg->ctxlen + xecfg->interhunkctxlen;
> +	long max_common = saturating_add(saturating_add(xecfg->ctxlen,
> +							xecfg->ctxlen),
> +					 xecfg->interhunkctxlen);

> René Scharfe <l.s.r@web.de> writes:
>
>>  t/t4055-diff-context.sh | 10 ++++++++++
>>  xdiff/xemit.c           |  8 +++++++-
>>  2 files changed, 17 insertions(+), 1 deletion(-)
>
> Oh, I love a patch like this that is well thought out to carefully
> check the bounds, instead of blindly say "ah, counting number of
> things in size_t solves everything" ;-)

> Am 14.03.25 um 23:28 schrieb Junio C Hamano:
>> René Scharfe <l.s.r@web.de> writes:
>>
>>>  t/t4055-diff-context.sh | 10 ++++++++++
>>>  xdiff/xemit.c           |  8 +++++++-
>>>  2 files changed, 17 insertions(+), 1 deletion(-)
>>
>> Oh, I love a patch like this that is well thought out to carefully
>> check the bounds, instead of blindly say "ah, counting number of
>> things in size_t solves everything" ;-)
>
> Converting xdiff from long to size_t is still a good idea, I think, ...

> René Scharfe <l.s.r@web.de> writes:
>
>> Comparisons to upstream
>> would become a lot more noisy as well.
>
> I am not sure how much of that matters these days, though.  Are they
> still active, or is the code perfect and pretty much done?  I somehow
> had the impression it has been the latter for a long time...

> xdl_get_hunk() calculates the maximum number of common lines between two
> changes that would fit into the same hunk for the given context options.
> It involves doubling and addition and thus can overflow if the terms are
> huge.
> 
> The type of ctxlen and interhunkctxlen in xdemitconf_t is long, while
> the type of the corresponding context and interhunkcontext in struct
> diff_options is int. On many platforms longs are bigger that ints,
> which prevents the overflow. On Windows they have the same range and
> the overflow manifests as hunks that are split erroneously and lines
> being repeated between them.
> 
> Fix the overflow by checking and not going beyond LONG_MAX. This allows
> specifying a huge context line count and getting all lines of a changed
> files in a single hunk, as expected.
> 
> Reported-by: Jason Cho jason11choca@proton.me
> 
> Signed-off-by: René Scharfe l.s.r@web.de
> 
> ---
> t/t4055-diff-context.sh | 10 ++++++++++
> xdiff/xemit.c | 8 +++++++-
> 2 files changed, 17 insertions(+), 1 deletion(-)
> 
> diff --git a/t/t4055-diff-context.sh b/t/t4055-diff-context.sh
> index f7ff234cf9..ec2804eea6 100755
> --- a/t/t4055-diff-context.sh
> +++ b/t/t4055-diff-context.sh
> @@ -89,4 +89,14 @@ test_expect_success '-U0 is valid, so is diff.context=0' '
> grep "^+MODIFIED" output
> '
> 
> +test_expect_success '-U2147483647 works' '
> + echo APPENDED >>x &&
> 
> + test_line_count = 16 x &&
> + git diff -U2147483647 >output &&
> 
> + test_line_count = 22 output &&
> + grep "^-ADDED" output &&
> + grep "^+MODIFIED" output &&
> + grep "^+APPENDED" output
> +'
> +
> test_done
> diff --git a/xdiff/xemit.c b/xdiff/xemit.c
> index f8e3f25b03..1d40c9cb40 100644
> --- a/xdiff/xemit.c
> +++ b/xdiff/xemit.c
> @@ -43,6 +43,10 @@ static int xdl_emit_record(xdfile_t *xdf, long ri, char const pre, xdemitcb_t *
> return 0;
> }
> 
> +static long saturating_add(long a, long b)
> +{
> + return signed_add_overflows(a, b) ? LONG_MAX : a + b;
> +}
> 
> /
> * Starting at the passed change atom, find the latest change atom to be included
> @@ -52,7 +56,9 @@ static int xdl_emit_record(xdfile_t *xdf, long ri, char const *pre, xdemitcb_t *
> xdchange_t *xdl_get_hunk(xdchange_t **xscr, xdemitconf_t const *xecfg)
> {
> xdchange_t *xch, *xchp, *lxch;
> - long max_common = 2 * xecfg->ctxlen + xecfg->interhunkctxlen;
> 
> + long max_common = saturating_add(saturating_add(xecfg->ctxlen,
> 
> + xecfg->ctxlen),
> 
> + xecfg->interhunkctxlen);
> 
> long max_ignorable = xecfg->ctxlen;
> 
> long ignored = 0; /* number of ignored blank lines */
> 
> --
> 2.48.1