From cca75eca0e81430bc966cad3506b0017652c2ab8 Mon Sep 17 00:00:00 2001 From: Michael Montalbo Date: Tue, 5 May 2026 13:57:24 -0700 Subject: [PATCH 1/4] diff: reject negative values for --inter-hunk-context 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 --- 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 397e38b41cc6fa..5df28e49c5057a 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_(""), diff --git a/diff.h b/diff.h index 7eb84aadf40c41..033d633db41129 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 bada0cbd32f764..bec1676f8d5a41 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 From f0478d434ce43e51fd92792c3e583df8a515bf05 Mon Sep 17 00:00:00 2001 From: Michael Montalbo Date: Tue, 5 May 2026 14:39:43 -0700 Subject: [PATCH 2/4] diff: reject negative values for -U/--unified 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 --- 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 5df28e49c5057a..1771b2c444c005 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 033d633db41129..bb5cddaf3499e9 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 1384a8195705e3..b26f6eea7c8332 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 && From f9cfa0c55dde8f7e876b568f8ea7caf555ffff1c Mon Sep 17 00:00:00 2001 From: Michael Montalbo Date: Tue, 5 May 2026 15:00:23 -0700 Subject: [PATCH 3/4] xdiff: guard against negative context lengths 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 --- xdiff/xemit.c | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/xdiff/xemit.c b/xdiff/xemit.c index 04f7e9193b61f0..7cd9cf0a44f4d6 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; From 05ff821e6ffec02a3bfc5aef542592de6a7add76 Mon Sep 17 00:00:00 2001 From: Michael Montalbo Date: Mon, 4 May 2026 23:14:56 -0700 Subject: [PATCH 4/4] parse-options: clarify PARSE_OPT_NONEG does not reject negative numbers 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 --- parse-options.h | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/parse-options.h b/parse-options.h index 706de9729f6b3f..c0a3a3dcae0535 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-