[PATCH 0/4] diff: reject negative context values

[PATCH 0/4] diff: reject negative context values

[Michael Montalbo via GitGitGadget]

Negative values for -U and --inter-hunk-context are silently accepted
and produce structurally invalid diff output.

Malformed hunk headers:

$ wc -l GIT-VERSION-GEN
106
$ git log -1 -p -U-500 -- GIT-VERSION-GEN | grep '^@@'                                                         
@@ -503,999- +503,999- @@


Line 503 of a 106-line file, count "999-" is not a valid integer.

Overlapping hunks that cannot be applied:

$ git log -1 -p -U3 --inter-hunk-context=100 791aeddfa2 \                                                      
    -- git-compat-util.h | git apply --check --reverse
(success)                                                                                                      
                                                                                                             
$ git log -1 -p -U3 --inter-hunk-context=-100 791aeddfa2 \                                                     
    -- git-compat-util.h | git apply --check --reverse                                                       
error: patch failed: git-compat-util.h:118                                                                     
error: git-compat-util.h: patch does not apply                                                               


Both options were originally parsed via opt_arg() which gated on
isdigit(), making negative values impossible. When they were converted
to OPT_INTEGER_F / OPT_CALLBACK in d473e2e0e8 (diff.c: convert
-U|--unified, 2019-01-27) and 16ed6c97cc (diff-parseopt: convert
--inter-hunk-context, 2019-03-24), the implicit rejection was lost.
PARSE_OPT_NONEG was added but only prevents the --no-* boolean form,
not negative numeric arguments.

This series restores the original invariant with stronger guarantees:

1/4  diff: reject negative values for --inter-hunk-context                                                     
     Change type to unsigned int, switch to OPT_UNSIGNED.                                                    
                                                                                                               
2/4  diff: reject negative values for -U/--unified                                                             
     Change type to unsigned int, add range check in callback.                                                 
                                                                                                               
3/4  xdiff: guard against negative context lengths                                                           
     BUG() in xdl_get_hunk() as defense in depth.
                                                                                                               
4/4  parse-options: clarify PARSE_OPT_NONEG does not reject                                                    
     negative numbers                                                                                          
     Documentation fix.                                                                                        


The config variables diff.context and diff.interHunkContext have
always rejected negative values. This series brings the CLI options in line.

Michael Montalbo (4):
  diff: reject negative values for --inter-hunk-context
  diff: reject negative values for -U/--unified
  xdiff: guard against negative context lengths
  parse-options: clarify PARSE_OPT_NONEG does not reject negative
    numbers

 diff.c                             | 25 ++++++++++++++-----------
 diff.h                             |  4 ++--
 parse-options.h                    |  5 ++++-
 t/t4032-diff-inter-hunk-context.sh |  6 ++++++
 t/t4055-diff-context.sh            |  5 +++++
 xdiff/xemit.c                      | 16 ++++++++++++----
 6 files changed, 43 insertions(+), 18 deletions(-)


base-commit: 94f057755b7941b321fd11fec1b2e3ca5313a4e0
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-2105%2Fmmontalbo%2Fmm%2Freject-negative-interhunk-context-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-2105/mmontalbo/mm/reject-negative-interhunk-context-v1
Pull-Request: https://github.com/gitgitgadget/git/pull/2105
-- 
gitgitgadget

[PATCH 1/4] diff: reject negative values for --inter-hunk-context

[Michael Montalbo via GitGitGadget]

From: Michael Montalbo <mmontalbo@gmail.com>

Negative values for --inter-hunk-context produce structurally invalid
diff output with overlapping hunks:

    $ git log -1 -p -U3 --inter-hunk-context=-100 791aeddfa2 \
        -- git-compat-util.h | grep '^@@'
    @@ -110,6 +110,9 @@
    @@ -115,6 +118,9 @@
    @@ -116,6 +122,7 @@

Hunk 1 covers lines 110-115, hunk 2 starts at 115 (overlap), hunk 3
starts at 116 (overlaps both). The resulting patch cannot be applied.

The config variable diff.interHunkContext already rejects negative
values, but the command line option does not. The option currently
uses OPT_INTEGER_F with PARSE_OPT_NONEG, but PARSE_OPT_NONEG only
prevents the "--no-inter-hunk-context" boolean negation form. It does
not reject negative numeric arguments like "--inter-hunk-context=-1".

Change the type of diff_options.interhunkcontext and its static
default from int to unsigned int, and switch the option parser from
OPT_INTEGER_F to OPT_UNSIGNED. This rejects negative values at parse
time via git_parse_unsigned() and enforces the correct type at compile
time via BARF_UNLESS_UNSIGNED.

Signed-off-by: Michael Montalbo <mmontalbo@gmail.com>
---
 diff.c                             | 13 ++++++-------
 diff.h                             |  2 +-
 t/t4032-diff-inter-hunk-context.sh |  6 ++++++
 3 files changed, 13 insertions(+), 8 deletions(-)

diff --git a/diff.c b/diff.c
index 397e38b41c..5df28e49c5 100644
--- a/diff.c
+++ b/diff.c
@@ -61,7 +61,7 @@ static enum git_colorbool diff_use_color_default = GIT_COLOR_UNKNOWN;
 static int diff_color_moved_default;
 static int diff_color_moved_ws_default;
 static int diff_context_default = 3;
-static int diff_interhunk_context_default;
+static unsigned int diff_interhunk_context_default;
 static char *diff_word_regex_cfg;
 static struct external_diff external_diff_cfg;
 static char *diff_order_file_cfg;
@@ -388,10 +388,10 @@ int git_diff_ui_config(const char *var, const char *value,
 		return 0;
 	}
 	if (!strcmp(var, "diff.interhunkcontext")) {
-		diff_interhunk_context_default = git_config_int(var, value,
-								ctx->kvi);
-		if (diff_interhunk_context_default < 0)
+		int val = git_config_int(var, value, ctx->kvi);
+		if (val < 0)
 			return -1;
+		diff_interhunk_context_default = val;
 		return 0;
 	}
 	if (!strcmp(var, "diff.renames")) {
@@ -6111,9 +6111,8 @@ struct option *add_diff_options(const struct option *opts,
 		OPT_CALLBACK_F(0, "default-prefix", options, NULL,
 			       N_("use default prefixes a/ and b/"),
 			       PARSE_OPT_NONEG | PARSE_OPT_NOARG, diff_opt_default_prefix),
-		OPT_INTEGER_F(0, "inter-hunk-context", &options->interhunkcontext,
-			      N_("show context between diff hunks up to the specified number of lines"),
-			      PARSE_OPT_NONEG),
+		OPT_UNSIGNED(0, "inter-hunk-context", &options->interhunkcontext,
+			     N_("show context between diff hunks up to the specified number of lines")),
 		OPT_CALLBACK_F(0, "output-indicator-new",
 			       &options->output_indicators[OUTPUT_INDICATOR_NEW],
 			       N_("<char>"),
diff --git a/diff.h b/diff.h
index 7eb84aadf4..033d633db4 100644
--- a/diff.h
+++ b/diff.h
@@ -296,7 +296,7 @@ struct diff_options {
 	/* Number of context lines to generate in patch output. */
 	int context;
 
-	int interhunkcontext;
+	unsigned int interhunkcontext;
 
 	/* Affects the way detection logic for complete rewrites, renames and
 	 * copies.
diff --git a/t/t4032-diff-inter-hunk-context.sh b/t/t4032-diff-inter-hunk-context.sh
index bada0cbd32..bec1676f8d 100755
--- a/t/t4032-diff-inter-hunk-context.sh
+++ b/t/t4032-diff-inter-hunk-context.sh
@@ -114,4 +114,10 @@ test_expect_success 'diff.interHunkContext invalid' '
 	test_must_fail git diff
 '
 
+test_expect_success '--inter-hunk-context rejects negative value' '
+	test_unconfig diff.interHunkContext &&
+	test_must_fail git diff --inter-hunk-context=-1 2>err &&
+	test_grep "expects a non-negative integer" err
+'
+
 test_done
-- 
gitgitgadget

[PATCH 2/4] diff: reject negative values for -U/--unified

[Michael Montalbo via GitGitGadget]

From: Michael Montalbo <mmontalbo@gmail.com>

Passing a negative value to -U is silently accepted and produces
corrupt unified diff output with malformed hunk headers:

    $ git log -1 -p -U-500 -- GIT-VERSION-GEN | grep '^@@'
    @@ -503,999- +503,999- @@

Line 503 of a 106-line file, count "999-" is not a valid integer.

The config variable diff.context already rejects negative values, but
the command line callback diff_opt_unified() uses strtol() with no
range check.

Change the type of diff_options.context and its static default from
int to unsigned int, matching the change to interhunkcontext in the
previous commit. The type change requires reworking the callback and
config parsing to validate in a local variable before assigning to
the now-unsigned field.

Unlike --inter-hunk-context which could be converted to OPT_UNSIGNED,
-U needs OPT_CALLBACK_F for PARSE_OPT_OPTARG (bare -U with no value
enables patch output). Add a range check in the callback instead.

Signed-off-by: Michael Montalbo <mmontalbo@gmail.com>
---
 diff.c                  | 12 ++++++++----
 diff.h                  |  2 +-
 t/t4055-diff-context.sh |  5 +++++
 3 files changed, 14 insertions(+), 5 deletions(-)

diff --git a/diff.c b/diff.c
index 5df28e49c5..1771b2c444 100644
--- a/diff.c
+++ b/diff.c
@@ -60,7 +60,7 @@ static int diff_suppress_blank_empty;
 static enum git_colorbool diff_use_color_default = GIT_COLOR_UNKNOWN;
 static int diff_color_moved_default;
 static int diff_color_moved_ws_default;
-static int diff_context_default = 3;
+static unsigned int diff_context_default = 3;
 static unsigned int diff_interhunk_context_default;
 static char *diff_word_regex_cfg;
 static struct external_diff external_diff_cfg;
@@ -382,9 +382,10 @@ int git_diff_ui_config(const char *var, const char *value,
 		return 0;
 	}
 	if (!strcmp(var, "diff.context")) {
-		diff_context_default = git_config_int(var, value, ctx->kvi);
-		if (diff_context_default < 0)
+		int val = git_config_int(var, value, ctx->kvi);
+		if (val < 0)
 			return -1;
+		diff_context_default = val;
 		return 0;
 	}
 	if (!strcmp(var, "diff.interhunkcontext")) {
@@ -5924,9 +5925,12 @@ static int diff_opt_unified(const struct option *opt,
 	BUG_ON_OPT_NEG(unset);
 
 	if (arg) {
-		options->context = strtol(arg, &s, 10);
+		long val = strtol(arg, &s, 10);
 		if (*s)
 			return error(_("%s expects a numerical value"), "--unified");
+		if (val < 0)
+			return error(_("%s expects a non-negative integer"), "--unified");
+		options->context = val;
 	}
 	enable_patch_output(&options->output_format);
 
diff --git a/diff.h b/diff.h
index 033d633db4..bb5cddaf34 100644
--- a/diff.h
+++ b/diff.h
@@ -294,7 +294,7 @@ struct diff_options {
 	enum git_colorbool use_color;
 
 	/* Number of context lines to generate in patch output. */
-	int context;
+	unsigned int context;
 
 	unsigned int interhunkcontext;
 
diff --git a/t/t4055-diff-context.sh b/t/t4055-diff-context.sh
index 1384a81957..b26f6eea7c 100755
--- a/t/t4055-diff-context.sh
+++ b/t/t4055-diff-context.sh
@@ -82,6 +82,11 @@ test_expect_success 'negative integer config parsing' '
 	test_grep "bad config variable" output
 '
 
+test_expect_success '-U-1 is rejected' '
+	test_must_fail git diff -U-1 2>err &&
+	test_grep "expects a non-negative integer" err
+'
+
 test_expect_success '-U0 is valid, so is diff.context=0' '
 	test_config diff.context 0 &&
 	git diff >output &&
-- 
gitgitgadget

[PATCH 3/4] xdiff: guard against negative context lengths

[Michael Montalbo via GitGitGadget]

From: Michael Montalbo <mmontalbo@gmail.com>

The xdemitconf_t fields ctxlen and interhunkctxlen are typed as long
(signed), but negative values are not meaningful for context line
counts. Unlike the diff_options fields changed in the previous two
commits, these cannot be converted to unsigned because the xdiff
arithmetic relies on signed subtraction:

    s1 = XDL_MAX(xch->i1 - xecfg->ctxlen, 0);

If ctxlen were unsigned long, the signed operand would be implicitly
converted to unsigned, and the subtraction would wrap to a large
positive value when i1 < ctxlen, defeating the XDL_MAX clamp. The
signed type is required for correct context-window calculations.

The previous two commits reject negative values at the parse layer
for --inter-hunk-context and -U/--unified, so negative values should
no longer reach xdiff in normal use. Add BUG() guards at the top of
xdl_get_hunk() as defense in depth to catch programming errors in
current or future callers that bypass option parsing.

xdl_get_hunk() is called by both xdl_emit_diff() and
xdl_call_hunk_func(), so a single guard covers all xdiff consumers.

Signed-off-by: Michael Montalbo <mmontalbo@gmail.com>
---
 xdiff/xemit.c | 16 ++++++++++++----
 1 file changed, 12 insertions(+), 4 deletions(-)

diff --git a/xdiff/xemit.c b/xdiff/xemit.c
index 04f7e9193b..7cd9cf0a44 100644
--- a/xdiff/xemit.c
+++ b/xdiff/xemit.c
@@ -46,12 +46,20 @@ static long saturating_add(long a, long b)
 xdchange_t *xdl_get_hunk(xdchange_t **xscr, xdemitconf_t const *xecfg)
 {
 	xdchange_t *xch, *xchp, *lxch;
-	long max_common = saturating_add(saturating_add(xecfg->ctxlen,
-							xecfg->ctxlen),
-					 xecfg->interhunkctxlen);
-	long max_ignorable = xecfg->ctxlen;
+	long max_common;
+	long max_ignorable;
 	long ignored = 0; /* number of ignored blank lines */
 
+	if (xecfg->ctxlen < 0)
+		BUG("negative context length: %ld", xecfg->ctxlen);
+	if (xecfg->interhunkctxlen < 0)
+		BUG("negative inter-hunk context length: %ld", xecfg->interhunkctxlen);
+
+	max_common = saturating_add(saturating_add(xecfg->ctxlen,
+						   xecfg->ctxlen),
+				    xecfg->interhunkctxlen);
+	max_ignorable = xecfg->ctxlen;
+
 	/* remove ignorable changes that are too far before other changes */
 	for (xchp = *xscr; xchp && xchp->ignore; xchp = xchp->next) {
 		xch = xchp->next;
-- 
gitgitgadget

[PATCH 4/4] parse-options: clarify PARSE_OPT_NONEG does not reject negative numbers

[Michael Montalbo via GitGitGadget]

From: Michael Montalbo <mmontalbo@gmail.com>

The name "NONEG" can be misread as "no negative [values]" when it
actually means "no [boolean] negation" (the --no-* form).

When --inter-hunk-context and -U/--unified were converted from a
custom parser to OPT_INTEGER_F with PARSE_OPT_NONEG in d473e2e0e8
and 16ed6c97cc, the implicit rejection of negative values (via
isdigit() in the old opt_arg() parser) was silently lost. The
previous commits in this series fix the resulting bugs.

Add a clarifying note to the flag documentation.

Signed-off-by: Michael Montalbo <mmontalbo@gmail.com>
---
 parse-options.h | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/parse-options.h b/parse-options.h
index 706de9729f..c0a3a3dcae 100644
--- a/parse-options.h
+++ b/parse-options.h
@@ -116,7 +116,10 @@ typedef int parse_opt_subcommand_fn(int argc, const char **argv,
  *   mask of parse_opt_option_flags.
  *   PARSE_OPT_OPTARG: says that the argument is optional (not for BOOLEANs)
  *   PARSE_OPT_NOARG: says that this option does not take an argument
- *   PARSE_OPT_NONEG: says that this option cannot be negated
+ *   PARSE_OPT_NONEG: says that this option cannot be negated (i.e.
+ *                   prevents --no-<option> boolean form). Does not reject
+ *                   negative numeric values like --option=-1. Use
+ *                   OPT_UNSIGNED for options that must be non-negative.
  *   PARSE_OPT_HIDDEN: this option is skipped in the default usage, and
  *                     shown only in the full usage.
  *   PARSE_OPT_LASTARG_DEFAULT: says that this option will take the default
-- 
gitgitgadget

Re: [PATCH 4/4] parse-options: clarify PARSE_OPT_NONEG does not reject negative numbers

[Junio C Hamano]

"Michael Montalbo via GitGitGadget" <gitgitgadget@gmail.com> writes:

> From: Michael Montalbo <mmontalbo@gmail.com>
>
> The name "NONEG" can be misread as "no negative [values]" when it
> actually means "no [boolean] negation" (the --no-* form).
>
> When --inter-hunk-context and -U/--unified were converted from a
> custom parser to OPT_INTEGER_F with PARSE_OPT_NONEG in d473e2e0e8
> and 16ed6c97cc, the implicit rejection of negative values (via
> isdigit() in the old opt_arg() parser) was silently lost. The
> previous commits in this series fix the resulting bugs.

I do not think _NONEG has anything to do with the bug.  It was
purely to reject --no-unified and --no-inter-hunk-context.

And there was no change to remove PARSE_OPT_NONEG from anywhere and
use OPT_UNSIGNED instead to fix any of the bugs fixed in this
series, ...

>
> Add a clarifying note to the flag documentation.
>
> Signed-off-by: Michael Montalbo <mmontalbo@gmail.com>
> ---
>  parse-options.h | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/parse-options.h b/parse-options.h
> index 706de9729f..c0a3a3dcae 100644
> --- a/parse-options.h
> +++ b/parse-options.h
> @@ -116,7 +116,10 @@ typedef int parse_opt_subcommand_fn(int argc, const char **argv,
>   *   mask of parse_opt_option_flags.
>   *   PARSE_OPT_OPTARG: says that the argument is optional (not for BOOLEANs)
>   *   PARSE_OPT_NOARG: says that this option does not take an argument
> - *   PARSE_OPT_NONEG: says that this option cannot be negated
> + *   PARSE_OPT_NONEG: says that this option cannot be negated (i.e.
> + *                   prevents --no-<option> boolean form). Does not reject
> + *                   negative numeric values like --option=-1. Use
> + *                   OPT_UNSIGNED for options that must be non-negative.

... I do not think the two additional sentences are warranted.  Stop
at clarifying what negated _means_ (i.e., rejects "--no-<option>"),
without adding what negated does _not_ mean.


>   *   PARSE_OPT_HIDDEN: this option is skipped in the default usage, and
>   *                     shown only in the full usage.
>   *   PARSE_OPT_LASTARG_DEFAULT: says that this option will take the default

Re: [PATCH 0/4] diff: reject negative context values

[Junio C Hamano]

"Michael Montalbo via GitGitGadget" <gitgitgadget@gmail.com> writes:

> Negative values for -U and --inter-hunk-context are silently accepted
> and produce structurally invalid diff output.
>
> Malformed hunk headers:
>
> $ wc -l GIT-VERSION-GEN
> 106
> $ git log -1 -p -U-500 -- GIT-VERSION-GEN | grep '^@@'                                                         
> @@ -503,999- +503,999- @@

It may not matter in the cover letter, but why do you need ~60
whitespace characters at the end of the command line, and many other
lines in the message?



>
>
> Line 503 of a 106-line file, count "999-" is not a valid integer.
>
> Overlapping hunks that cannot be applied:
>
> $ git log -1 -p -U3 --inter-hunk-context=100 791aeddfa2 \                                                      
>     -- git-compat-util.h | git apply --check --reverse
> (success)                                                                                                      
>                                                                                                              
> $ git log -1 -p -U3 --inter-hunk-context=-100 791aeddfa2 \                                                     
>     -- git-compat-util.h | git apply --check --reverse                                                       
> error: patch failed: git-compat-util.h:118                                                                     
> error: git-compat-util.h: patch does not apply                                                               
>
>
> Both options were originally parsed via opt_arg() which gated on
> isdigit(), making negative values impossible. When they were converted
> to OPT_INTEGER_F / OPT_CALLBACK in d473e2e0e8 (diff.c: convert
> -U|--unified, 2019-01-27) and 16ed6c97cc (diff-parseopt: convert
> --inter-hunk-context, 2019-03-24), the implicit rejection was lost.
> PARSE_OPT_NONEG was added but only prevents the --no-* boolean form,
> not negative numeric arguments.
>
> This series restores the original invariant with stronger guarantees:
>
> 1/4  diff: reject negative values for --inter-hunk-context                                                     
>      Change type to unsigned int, switch to OPT_UNSIGNED.                                                    
>                                                                                                                
> 2/4  diff: reject negative values for -U/--unified                                                             
>      Change type to unsigned int, add range check in callback.                                                 
>                                                                                                                
> 3/4  xdiff: guard against negative context lengths                                                           
>      BUG() in xdl_get_hunk() as defense in depth.
>                                                                                                                
> 4/4  parse-options: clarify PARSE_OPT_NONEG does not reject                                                    
>      negative numbers                                                                                          
>      Documentation fix.                                                                                        
>
>
> The config variables diff.context and diff.interHunkContext have
> always rejected negative values. This series brings the CLI options in line.
>
> Michael Montalbo (4):
>   diff: reject negative values for --inter-hunk-context
>   diff: reject negative values for -U/--unified
>   xdiff: guard against negative context lengths
>   parse-options: clarify PARSE_OPT_NONEG does not reject negative
>     numbers
>
>  diff.c                             | 25 ++++++++++++++-----------
>  diff.h                             |  4 ++--
>  parse-options.h                    |  5 ++++-
>  t/t4032-diff-inter-hunk-context.sh |  6 ++++++
>  t/t4055-diff-context.sh            |  5 +++++
>  xdiff/xemit.c                      | 16 ++++++++++++----
>  6 files changed, 43 insertions(+), 18 deletions(-)
>
>
> base-commit: 94f057755b7941b321fd11fec1b2e3ca5313a4e0
> Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-2105%2Fmmontalbo%2Fmm%2Freject-negative-interhunk-context-v1
> Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-2105/mmontalbo/mm/reject-negative-interhunk-context-v1
> Pull-Request: https://github.com/gitgitgadget/git/pull/2105

Re: [PATCH 4/4] parse-options: clarify PARSE_OPT_NONEG does not reject negative numbers

[Michael Montalbo]

On Sat, May 9, 2026 at 3:01 PM Junio C Hamano <gitster@pobox.com> wrote:
>
> "Michael Montalbo via GitGitGadget" <gitgitgadget@gmail.com> writes:
>
> > From: Michael Montalbo <mmontalbo@gmail.com>
> >
> > The name "NONEG" can be misread as "no negative [values]" when it
> > actually means "no [boolean] negation" (the --no-* form).
> >
> > When --inter-hunk-context and -U/--unified were converted from a
> > custom parser to OPT_INTEGER_F with PARSE_OPT_NONEG in d473e2e0e8
> > and 16ed6c97cc, the implicit rejection of negative values (via
> > isdigit() in the old opt_arg() parser) was silently lost. The
> > previous commits in this series fix the resulting bugs.
>
> I do not think _NONEG has anything to do with the bug.  It was
> purely to reject --no-unified and --no-inter-hunk-context.
>

You are right this was a mistaken assumption on my part.

> And there was no change to remove PARSE_OPT_NONEG from anywhere and
> use OPT_UNSIGNED instead to fix any of the bugs fixed in this
> series, ...
>

Right, PARSE_OPT_NONEG is still enabled implicitly by OPT_UNSIGNED so
PARSE_OPT_NONEG really has no part in the story.

> >
> > Add a clarifying note to the flag documentation.
> >
> > Signed-off-by: Michael Montalbo <mmontalbo@gmail.com>
> > ---
> >  parse-options.h | 5 ++++-
> >  1 file changed, 4 insertions(+), 1 deletion(-)
> >
> > diff --git a/parse-options.h b/parse-options.h
> > index 706de9729f..c0a3a3dcae 100644
> > --- a/parse-options.h
> > +++ b/parse-options.h
> > @@ -116,7 +116,10 @@ typedef int parse_opt_subcommand_fn(int argc, const char **argv,
> >   *   mask of parse_opt_option_flags.
> >   *   PARSE_OPT_OPTARG: says that the argument is optional (not for BOOLEANs)
> >   *   PARSE_OPT_NOARG: says that this option does not take an argument
> > - *   PARSE_OPT_NONEG: says that this option cannot be negated
> > + *   PARSE_OPT_NONEG: says that this option cannot be negated (i.e.
> > + *                   prevents --no-<option> boolean form). Does not reject
> > + *                   negative numeric values like --option=-1. Use
> > + *                   OPT_UNSIGNED for options that must be non-negative.
>
> ... I do not think the two additional sentences are warranted.  Stop
> at clarifying what negated _means_ (i.e., rejects "--no-<option>"),
> without adding what negated does _not_ mean.
>

Ok, will remove the latter portion of the change in a follow-up.

>
> >   *   PARSE_OPT_HIDDEN: this option is skipped in the default usage, and
> >   *                     shown only in the full usage.
> >   *   PARSE_OPT_LASTARG_DEFAULT: says that this option will take the default

Re: [PATCH 0/4] diff: reject negative context values

[Michael Montalbo]

On Sat, May 9, 2026 at 6:01 PM Junio C Hamano <gitster@pobox.com> wrote:
>
> "Michael Montalbo via GitGitGadget" <gitgitgadget@gmail.com> writes:
>
> > Negative values for -U and --inter-hunk-context are silently accepted
> > and produce structurally invalid diff output.
> >
> > Malformed hunk headers:
> >
> > $ wc -l GIT-VERSION-GEN
> > 106
> > $ git log -1 -p -U-500 -- GIT-VERSION-GEN | grep '^@@'
> > @@ -503,999- +503,999- @@
>
> It may not matter in the cover letter, but why do you need ~60
> whitespace characters at the end of the command line, and many other
> lines in the message?
>

Apologies, this is a mistake I made between formatting my cover letter
and moving
it to GitGitGadget. I will clean up the cover letter and be more
careful about that in
the future. Thank you for pointing this out.

>
>
> >
> >
> > Line 503 of a 106-line file, count "999-" is not a valid integer.
> >
> > Overlapping hunks that cannot be applied:
> >
> > $ git log -1 -p -U3 --inter-hunk-context=100 791aeddfa2 \
> >     -- git-compat-util.h | git apply --check --reverse
> > (success)
> >
> > $ git log -1 -p -U3 --inter-hunk-context=-100 791aeddfa2 \
> >     -- git-compat-util.h | git apply --check --reverse
> > error: patch failed: git-compat-util.h:118
> > error: git-compat-util.h: patch does not apply
> >
> >
> > Both options were originally parsed via opt_arg() which gated on
> > isdigit(), making negative values impossible. When they were converted
> > to OPT_INTEGER_F / OPT_CALLBACK in d473e2e0e8 (diff.c: convert
> > -U|--unified, 2019-01-27) and 16ed6c97cc (diff-parseopt: convert
> > --inter-hunk-context, 2019-03-24), the implicit rejection was lost.
> > PARSE_OPT_NONEG was added but only prevents the --no-* boolean form,
> > not negative numeric arguments.
> >
> > This series restores the original invariant with stronger guarantees:
> >
> > 1/4  diff: reject negative values for --inter-hunk-context
> >      Change type to unsigned int, switch to OPT_UNSIGNED.
> >
> > 2/4  diff: reject negative values for -U/--unified
> >      Change type to unsigned int, add range check in callback.
> >
> > 3/4  xdiff: guard against negative context lengths
> >      BUG() in xdl_get_hunk() as defense in depth.
> >
> > 4/4  parse-options: clarify PARSE_OPT_NONEG does not reject
> >      negative numbers
> >      Documentation fix.
> >
> >
> > The config variables diff.context and diff.interHunkContext have
> > always rejected negative values. This series brings the CLI options in line.
> >
> > Michael Montalbo (4):
> >   diff: reject negative values for --inter-hunk-context
> >   diff: reject negative values for -U/--unified
> >   xdiff: guard against negative context lengths
> >   parse-options: clarify PARSE_OPT_NONEG does not reject negative
> >     numbers
> >
> >  diff.c                             | 25 ++++++++++++++-----------
> >  diff.h                             |  4 ++--
> >  parse-options.h                    |  5 ++++-
> >  t/t4032-diff-inter-hunk-context.sh |  6 ++++++
> >  t/t4055-diff-context.sh            |  5 +++++
> >  xdiff/xemit.c                      | 16 ++++++++++++----
> >  6 files changed, 43 insertions(+), 18 deletions(-)
> >
> >
> > base-commit: 94f057755b7941b321fd11fec1b2e3ca5313a4e0
> > Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-2105%2Fmmontalbo%2Fmm%2Freject-negative-interhunk-context-v1
> > Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-2105/mmontalbo/mm/reject-negative-interhunk-context-v1
> > Pull-Request: https://github.com/gitgitgadget/git/pull/2105

[PATCH v2 0/4] diff: reject negative context values

[Michael Montalbo via GitGitGadget]

Negative values for -U and --inter-hunk-context are silently accepted and
produce structurally invalid diff output.

Malformed hunk headers:

$ wc -l GIT-VERSION-GEN 106 $ git log -1 -p -U-500 -- GIT-VERSION-GEN | grep
'^@@' @@ -503,999- +503,999- @@

Line 503 of a 106-line file, count "999-" is not a valid integer.

Overlapping hunks that cannot be applied:

$ git log -1 -p -U3 --inter-hunk-context=100 791aeddfa2
-- git-compat-util.h | git apply --check --reverse (success)

$ git log -1 -p -U3 --inter-hunk-context=-100 791aeddfa2
-- git-compat-util.h | git apply --check --reverse error: patch failed:
git-compat-util.h:118 error: git-compat-util.h: patch does not apply

Both options were originally parsed via opt_arg() which gated on isdigit(),
making negative values impossible. When they were converted to OPT_INTEGER_F
/ OPT_CALLBACK in d473e2e0e8 (diff.c: convert -U|--unified, 2019-01-27) and
16ed6c97cc (diff-parseopt: convert --inter-hunk-context, 2019-03-24), the
implicit rejection was lost.

This series restores the original invariant with stronger guarantees:

1/4 diff: reject negative values for --inter-hunk-context Change type to
unsigned int, switch to OPT_UNSIGNED.

2/4 diff: reject negative values for -U/--unified Change type to unsigned
int, add range check in callback.

3/4 xdiff: guard against negative context lengths BUG() in xdl_get_hunk() as
defense in depth.

4/4 parse-options: clarify what "negated" means for PARSE_OPT_NONEG.

The config variables diff.context and diff.interHunkContext have always
rejected negative values. This series brings the CLI options in line.

Changes since v1:

Patch 1 and 4: Rewrote commit message to not imply NONEG was related to the
bug.

Patch 4: Trimmed to just clarify what "negated" means, without documenting
what PARSE_OPT_NONEG does not do.

Michael Montalbo (4):
  diff: reject negative values for --inter-hunk-context
  diff: reject negative values for -U/--unified
  xdiff: guard against negative context lengths
  parse-options: clarify what "negated" means for PARSE_OPT_NONEG

 diff.c                             | 25 ++++++++++++++-----------
 diff.h                             |  4 ++--
 parse-options.h                    |  1 +
 t/t4032-diff-inter-hunk-context.sh |  6 ++++++
 t/t4055-diff-context.sh            |  5 +++++
 xdiff/xemit.c                      | 16 ++++++++++++----
 6 files changed, 40 insertions(+), 17 deletions(-)


base-commit: 94f057755b7941b321fd11fec1b2e3ca5313a4e0
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-2105%2Fmmontalbo%2Fmm%2Freject-negative-interhunk-context-v2
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-2105/mmontalbo/mm/reject-negative-interhunk-context-v2
Pull-Request: https://github.com/gitgitgadget/git/pull/2105

Range-diff vs v1:

 1:  cca75eca0e ! 1:  f2ebb3a72b diff: reject negative values for --inter-hunk-context
     @@ Commit message
          starts at 116 (overlaps both). The resulting patch cannot be applied.
      
          The config variable diff.interHunkContext already rejects negative
     -    values, but the command line option does not. The option currently
     -    uses OPT_INTEGER_F with PARSE_OPT_NONEG, but PARSE_OPT_NONEG only
     -    prevents the "--no-inter-hunk-context" boolean negation form. It does
     -    not reject negative numeric arguments like "--inter-hunk-context=-1".
     +    values, but the command line option does not.
      
          Change the type of diff_options.interhunkcontext and its static
          default from int to unsigned int, and switch the option parser from
 2:  f0478d434c = 2:  fc3d2bc31e diff: reject negative values for -U/--unified
 3:  f9cfa0c55d = 3:  020ca774c0 xdiff: guard against negative context lengths
 4:  05ff821e6f ! 4:  3a656f8c0f parse-options: clarify PARSE_OPT_NONEG does not reject negative numbers
     @@ Metadata
      Author: Michael Montalbo <mmontalbo@gmail.com>
      
       ## Commit message ##
     -    parse-options: clarify PARSE_OPT_NONEG does not reject negative numbers
     +    parse-options: clarify what "negated" means for PARSE_OPT_NONEG
      
     -    The name "NONEG" can be misread as "no negative [values]" when it
     -    actually means "no [boolean] negation" (the --no-* form).
     -
     -    When --inter-hunk-context and -U/--unified were converted from a
     -    custom parser to OPT_INTEGER_F with PARSE_OPT_NONEG in d473e2e0e8
     -    and 16ed6c97cc, the implicit rejection of negative values (via
     -    isdigit() in the old opt_arg() parser) was silently lost. The
     -    previous commits in this series fix the resulting bugs.
     -
     -    Add a clarifying note to the flag documentation.
     +    The documentation says the flag prevents an option from being
     +    "negated" without specifying what that means. Add a parenthetical
     +    to clarify that it rejects the "--no-<option>" form.
      
          Signed-off-by: Michael Montalbo <mmontalbo@gmail.com>
      
       ## parse-options.h ##
      @@ parse-options.h: typedef int parse_opt_subcommand_fn(int argc, const char **argv,
     -  *   mask of parse_opt_option_flags.
        *   PARSE_OPT_OPTARG: says that the argument is optional (not for BOOLEANs)
        *   PARSE_OPT_NOARG: says that this option does not take an argument
     -- *   PARSE_OPT_NONEG: says that this option cannot be negated
     -+ *   PARSE_OPT_NONEG: says that this option cannot be negated (i.e.
     -+ *                   prevents --no-<option> boolean form). Does not reject
     -+ *                   negative numeric values like --option=-1. Use
     -+ *                   OPT_UNSIGNED for options that must be non-negative.
     +  *   PARSE_OPT_NONEG: says that this option cannot be negated
     ++ *                   (i.e. rejects "--no-<option>")
        *   PARSE_OPT_HIDDEN: this option is skipped in the default usage, and
        *                     shown only in the full usage.
        *   PARSE_OPT_LASTARG_DEFAULT: says that this option will take the default

-- 
gitgitgadget

[PATCH v2 1/4] diff: reject negative values for --inter-hunk-context

[Michael Montalbo via GitGitGadget]

From: Michael Montalbo <mmontalbo@gmail.com>

Negative values for --inter-hunk-context produce structurally invalid
diff output with overlapping hunks:

    $ git log -1 -p -U3 --inter-hunk-context=-100 791aeddfa2 \
        -- git-compat-util.h | grep '^@@'
    @@ -110,6 +110,9 @@
    @@ -115,6 +118,9 @@
    @@ -116,6 +122,7 @@

Hunk 1 covers lines 110-115, hunk 2 starts at 115 (overlap), hunk 3
starts at 116 (overlaps both). The resulting patch cannot be applied.

The config variable diff.interHunkContext already rejects negative
values, but the command line option does not.

Change the type of diff_options.interhunkcontext and its static
default from int to unsigned int, and switch the option parser from
OPT_INTEGER_F to OPT_UNSIGNED. This rejects negative values at parse
time via git_parse_unsigned() and enforces the correct type at compile
time via BARF_UNLESS_UNSIGNED.

Signed-off-by: Michael Montalbo <mmontalbo@gmail.com>
---
 diff.c                             | 13 ++++++-------
 diff.h                             |  2 +-
 t/t4032-diff-inter-hunk-context.sh |  6 ++++++
 3 files changed, 13 insertions(+), 8 deletions(-)

diff --git a/diff.c b/diff.c
index 397e38b41c..5df28e49c5 100644
--- a/diff.c
+++ b/diff.c
@@ -61,7 +61,7 @@ static enum git_colorbool diff_use_color_default = GIT_COLOR_UNKNOWN;
 static int diff_color_moved_default;
 static int diff_color_moved_ws_default;
 static int diff_context_default = 3;
-static int diff_interhunk_context_default;
+static unsigned int diff_interhunk_context_default;
 static char *diff_word_regex_cfg;
 static struct external_diff external_diff_cfg;
 static char *diff_order_file_cfg;
@@ -388,10 +388,10 @@ int git_diff_ui_config(const char *var, const char *value,
 		return 0;
 	}
 	if (!strcmp(var, "diff.interhunkcontext")) {
-		diff_interhunk_context_default = git_config_int(var, value,
-								ctx->kvi);
-		if (diff_interhunk_context_default < 0)
+		int val = git_config_int(var, value, ctx->kvi);
+		if (val < 0)
 			return -1;
+		diff_interhunk_context_default = val;
 		return 0;
 	}
 	if (!strcmp(var, "diff.renames")) {
@@ -6111,9 +6111,8 @@ struct option *add_diff_options(const struct option *opts,
 		OPT_CALLBACK_F(0, "default-prefix", options, NULL,
 			       N_("use default prefixes a/ and b/"),
 			       PARSE_OPT_NONEG | PARSE_OPT_NOARG, diff_opt_default_prefix),
-		OPT_INTEGER_F(0, "inter-hunk-context", &options->interhunkcontext,
-			      N_("show context between diff hunks up to the specified number of lines"),
-			      PARSE_OPT_NONEG),
+		OPT_UNSIGNED(0, "inter-hunk-context", &options->interhunkcontext,
+			     N_("show context between diff hunks up to the specified number of lines")),
 		OPT_CALLBACK_F(0, "output-indicator-new",
 			       &options->output_indicators[OUTPUT_INDICATOR_NEW],
 			       N_("<char>"),
diff --git a/diff.h b/diff.h
index 7eb84aadf4..033d633db4 100644
--- a/diff.h
+++ b/diff.h
@@ -296,7 +296,7 @@ struct diff_options {
 	/* Number of context lines to generate in patch output. */
 	int context;
 
-	int interhunkcontext;
+	unsigned int interhunkcontext;
 
 	/* Affects the way detection logic for complete rewrites, renames and
 	 * copies.
diff --git a/t/t4032-diff-inter-hunk-context.sh b/t/t4032-diff-inter-hunk-context.sh
index bada0cbd32..bec1676f8d 100755
--- a/t/t4032-diff-inter-hunk-context.sh
+++ b/t/t4032-diff-inter-hunk-context.sh
@@ -114,4 +114,10 @@ test_expect_success 'diff.interHunkContext invalid' '
 	test_must_fail git diff
 '
 
+test_expect_success '--inter-hunk-context rejects negative value' '
+	test_unconfig diff.interHunkContext &&
+	test_must_fail git diff --inter-hunk-context=-1 2>err &&
+	test_grep "expects a non-negative integer" err
+'
+
 test_done
-- 
gitgitgadget

[PATCH v2 2/4] diff: reject negative values for -U/--unified

[Michael Montalbo via GitGitGadget]

From: Michael Montalbo <mmontalbo@gmail.com>

Passing a negative value to -U is silently accepted and produces
corrupt unified diff output with malformed hunk headers:

    $ git log -1 -p -U-500 -- GIT-VERSION-GEN | grep '^@@'
    @@ -503,999- +503,999- @@

Line 503 of a 106-line file, count "999-" is not a valid integer.

The config variable diff.context already rejects negative values, but
the command line callback diff_opt_unified() uses strtol() with no
range check.

Change the type of diff_options.context and its static default from
int to unsigned int, matching the change to interhunkcontext in the
previous commit. The type change requires reworking the callback and
config parsing to validate in a local variable before assigning to
the now-unsigned field.

Unlike --inter-hunk-context which could be converted to OPT_UNSIGNED,
-U needs OPT_CALLBACK_F for PARSE_OPT_OPTARG (bare -U with no value
enables patch output). Add a range check in the callback instead.

Signed-off-by: Michael Montalbo <mmontalbo@gmail.com>
---
 diff.c                  | 12 ++++++++----
 diff.h                  |  2 +-
 t/t4055-diff-context.sh |  5 +++++
 3 files changed, 14 insertions(+), 5 deletions(-)

diff --git a/diff.c b/diff.c
index 5df28e49c5..1771b2c444 100644
--- a/diff.c
+++ b/diff.c
@@ -60,7 +60,7 @@ static int diff_suppress_blank_empty;
 static enum git_colorbool diff_use_color_default = GIT_COLOR_UNKNOWN;
 static int diff_color_moved_default;
 static int diff_color_moved_ws_default;
-static int diff_context_default = 3;
+static unsigned int diff_context_default = 3;
 static unsigned int diff_interhunk_context_default;
 static char *diff_word_regex_cfg;
 static struct external_diff external_diff_cfg;
@@ -382,9 +382,10 @@ int git_diff_ui_config(const char *var, const char *value,
 		return 0;
 	}
 	if (!strcmp(var, "diff.context")) {
-		diff_context_default = git_config_int(var, value, ctx->kvi);
-		if (diff_context_default < 0)
+		int val = git_config_int(var, value, ctx->kvi);
+		if (val < 0)
 			return -1;
+		diff_context_default = val;
 		return 0;
 	}
 	if (!strcmp(var, "diff.interhunkcontext")) {
@@ -5924,9 +5925,12 @@ static int diff_opt_unified(const struct option *opt,
 	BUG_ON_OPT_NEG(unset);
 
 	if (arg) {
-		options->context = strtol(arg, &s, 10);
+		long val = strtol(arg, &s, 10);
 		if (*s)
 			return error(_("%s expects a numerical value"), "--unified");
+		if (val < 0)
+			return error(_("%s expects a non-negative integer"), "--unified");
+		options->context = val;
 	}
 	enable_patch_output(&options->output_format);
 
diff --git a/diff.h b/diff.h
index 033d633db4..bb5cddaf34 100644
--- a/diff.h
+++ b/diff.h
@@ -294,7 +294,7 @@ struct diff_options {
 	enum git_colorbool use_color;
 
 	/* Number of context lines to generate in patch output. */
-	int context;
+	unsigned int context;
 
 	unsigned int interhunkcontext;
 
diff --git a/t/t4055-diff-context.sh b/t/t4055-diff-context.sh
index 1384a81957..b26f6eea7c 100755
--- a/t/t4055-diff-context.sh
+++ b/t/t4055-diff-context.sh
@@ -82,6 +82,11 @@ test_expect_success 'negative integer config parsing' '
 	test_grep "bad config variable" output
 '
 
+test_expect_success '-U-1 is rejected' '
+	test_must_fail git diff -U-1 2>err &&
+	test_grep "expects a non-negative integer" err
+'
+
 test_expect_success '-U0 is valid, so is diff.context=0' '
 	test_config diff.context 0 &&
 	git diff >output &&
-- 
gitgitgadget

[PATCH v2 3/4] xdiff: guard against negative context lengths

[Michael Montalbo via GitGitGadget]

From: Michael Montalbo <mmontalbo@gmail.com>

The xdemitconf_t fields ctxlen and interhunkctxlen are typed as long
(signed), but negative values are not meaningful for context line
counts. Unlike the diff_options fields changed in the previous two
commits, these cannot be converted to unsigned because the xdiff
arithmetic relies on signed subtraction:

    s1 = XDL_MAX(xch->i1 - xecfg->ctxlen, 0);

If ctxlen were unsigned long, the signed operand would be implicitly
converted to unsigned, and the subtraction would wrap to a large
positive value when i1 < ctxlen, defeating the XDL_MAX clamp. The
signed type is required for correct context-window calculations.

The previous two commits reject negative values at the parse layer
for --inter-hunk-context and -U/--unified, so negative values should
no longer reach xdiff in normal use. Add BUG() guards at the top of
xdl_get_hunk() as defense in depth to catch programming errors in
current or future callers that bypass option parsing.

xdl_get_hunk() is called by both xdl_emit_diff() and
xdl_call_hunk_func(), so a single guard covers all xdiff consumers.

Signed-off-by: Michael Montalbo <mmontalbo@gmail.com>
---
 xdiff/xemit.c | 16 ++++++++++++----
 1 file changed, 12 insertions(+), 4 deletions(-)

diff --git a/xdiff/xemit.c b/xdiff/xemit.c
index 04f7e9193b..7cd9cf0a44 100644
--- a/xdiff/xemit.c
+++ b/xdiff/xemit.c
@@ -46,12 +46,20 @@ static long saturating_add(long a, long b)
 xdchange_t *xdl_get_hunk(xdchange_t **xscr, xdemitconf_t const *xecfg)
 {
 	xdchange_t *xch, *xchp, *lxch;
-	long max_common = saturating_add(saturating_add(xecfg->ctxlen,
-							xecfg->ctxlen),
-					 xecfg->interhunkctxlen);
-	long max_ignorable = xecfg->ctxlen;
+	long max_common;
+	long max_ignorable;
 	long ignored = 0; /* number of ignored blank lines */
 
+	if (xecfg->ctxlen < 0)
+		BUG("negative context length: %ld", xecfg->ctxlen);
+	if (xecfg->interhunkctxlen < 0)
+		BUG("negative inter-hunk context length: %ld", xecfg->interhunkctxlen);
+
+	max_common = saturating_add(saturating_add(xecfg->ctxlen,
+						   xecfg->ctxlen),
+				    xecfg->interhunkctxlen);
+	max_ignorable = xecfg->ctxlen;
+
 	/* remove ignorable changes that are too far before other changes */
 	for (xchp = *xscr; xchp && xchp->ignore; xchp = xchp->next) {
 		xch = xchp->next;
-- 
gitgitgadget

[PATCH v2 4/4] parse-options: clarify what "negated" means for PARSE_OPT_NONEG

[Michael Montalbo via GitGitGadget]

From: Michael Montalbo <mmontalbo@gmail.com>

The documentation says the flag prevents an option from being
"negated" without specifying what that means. Add a parenthetical
to clarify that it rejects the "--no-<option>" form.

Signed-off-by: Michael Montalbo <mmontalbo@gmail.com>
---
 parse-options.h | 1 +
 1 file changed, 1 insertion(+)

diff --git a/parse-options.h b/parse-options.h
index 706de9729f..0d1f738f8d 100644
--- a/parse-options.h
+++ b/parse-options.h
@@ -117,6 +117,7 @@ typedef int parse_opt_subcommand_fn(int argc, const char **argv,
  *   PARSE_OPT_OPTARG: says that the argument is optional (not for BOOLEANs)
  *   PARSE_OPT_NOARG: says that this option does not take an argument
  *   PARSE_OPT_NONEG: says that this option cannot be negated
+ *                   (i.e. rejects "--no-<option>")
  *   PARSE_OPT_HIDDEN: this option is skipped in the default usage, and
  *                     shown only in the full usage.
  *   PARSE_OPT_LASTARG_DEFAULT: says that this option will take the default
-- 
gitgitgadget

Re: [PATCH v2 0/4] diff: reject negative context values

[Junio C Hamano]

"Michael Montalbo via GitGitGadget" <gitgitgadget@gmail.com> writes:

> Changes since v1:
>
> Patch 1 and 4: Rewrote commit message to not imply NONEG was related to the
> bug.
>
> Patch 4: Trimmed to just clarify what "negated" means, without documenting
> what PARSE_OPT_NONEG does not do.

Thanks.  Will queue.  I have nothing more to add, but I will hold
off on marking it for 'next' to give others a chance to comment.