Skip to content

diff: reject negative context values #2105

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
mmontalbo wants to merge 4 commits into
base: master
Choose a base branch
from
Open

diff: reject negative context values #2105

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
cca75ec
diff: reject negative values for --inter-hunk-context
mmontalbo May 5, 2026
f0478d4
diff: reject negative values for -U/--unified
mmontalbo May 5, 2026
f9cfa0c
xdiff: guard against negative context lengths
mmontalbo May 5, 2026
05ff821
parse-options: clarify PARSE_OPT_NONEG does not reject negative numbers
mmontalbo May 5, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 14 additions & 11 deletions diff.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -60,8 +60,8 @@ 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 int diff_interhunk_context_default;
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;
static char *diff_order_file_cfg;
Expand Down Expand Up @@ -382,16 +382,17 @@ 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")) {
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")) {
Expand Down Expand Up @@ -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);

Expand Down Expand Up @@ -6111,9 +6115,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>"),
Expand Down
4 changes: 2 additions & 2 deletions diff.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -294,9 +294,9 @@ struct diff_options {
enum git_colorbool use_color;

/* Number of context lines to generate in patch output. */
int context;
unsigned int context;

int interhunkcontext;
unsigned int interhunkcontext;

/* Affects the way detection logic for complete rewrites, renames and
* copies.
Expand Down
5 changes: 4 additions & 1 deletion parse-options.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -116,7 +116,10 @@ typedef int parse_opt_subcommand_fn(int argc, const char **argv,
* mask of parse_opt_option_flags.
Copy link
Copy Markdown

@gitgitgadget gitgitgadget Bot May 9, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Junio C Hamano wrote on the Git mailing list (how to reply to this email):

"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

* 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
Expand Down
6 changes: 6 additions & 0 deletions t/t4032-diff-inter-hunk-context.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
5 changes: 5 additions & 0 deletions t/t4055-diff-context.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 &&
Expand Down
16 changes: 12 additions & 4 deletions xdiff/xemit.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
Loading