diff --git a/0001-diff-parseopt-correct-variable-types-that-are-used-b.patch b/0001-diff-parseopt-correct-variable-types-that-are-used-b.patch new file mode 100644 index 0000000..be0e181 --- /dev/null +++ b/0001-diff-parseopt-correct-variable-types-that-are-used-b.patch @@ -0,0 +1,128 @@ +From 64144ae182fb4f01fbbce0e3005057ff95aacc58 Mon Sep 17 00:00:00 2001 +From: =?UTF-8?q?Nguy=E1=BB=85n=20Th=C3=A1i=20Ng=E1=BB=8Dc=20Duy?= + +Date: Fri, 24 May 2019 16:24:40 +0700 +Subject: [PATCH 1/3] diff-parseopt: correct variable types that are used by + parseopt +MIME-Version: 1.0 +Content-Type: text/plain; charset=UTF-8 +Content-Transfer-Encoding: 8bit + +Most number-related OPT_ macros store the value in an 'int' +variable. Many of the variables in 'struct diff_options' have a +different type, but during the conversion to using parse_options() I +failed to notice and correct. + +The problem was reported on s360x which is a big-endian +architechture. The variable to store '-w' option in this case is +xdl_opts, 'long' type, 8 bytes. But since parse_options() assumes +'int' (4 bytes), it will store bits in the wrong part of xdl_opts. The +problem was found on little-endian platforms because parse_options() +will accidentally store at the right part of xdl_opts. + +There aren't much to say about the type change (except that 'int' for +xdl_opts should still be big enough, since Windows' long is the same +size as 'int' and nobody has complained so far). Some safety checks may +be implemented in the future to prevent class of bugs. + +Reported-by: Todd Zullinger +Signed-off-by: Nguyễn Thái Ngọc Duy +--- + diff.h | 70 +++++++++++++++++++++++++++++----------------------------- + 1 file changed, 35 insertions(+), 35 deletions(-) + +diff --git a/diff.h b/diff.h +index b20cbcc091..4527daf6b7 100644 +--- a/diff.h ++++ b/diff.h +@@ -65,39 +65,39 @@ typedef struct strbuf *(*diff_prefix_fn_t)(struct diff_options *opt, void *data) + + #define DIFF_FLAGS_INIT { 0 } + struct diff_flags { +- unsigned recursive; +- unsigned tree_in_recursive; +- unsigned binary; +- unsigned text; +- unsigned full_index; +- unsigned silent_on_remove; +- unsigned find_copies_harder; +- unsigned follow_renames; +- unsigned rename_empty; +- unsigned has_changes; +- unsigned quick; +- unsigned no_index; +- unsigned allow_external; +- unsigned exit_with_status; +- unsigned reverse_diff; +- unsigned check_failed; +- unsigned relative_name; +- unsigned ignore_submodules; +- unsigned dirstat_cumulative; +- unsigned dirstat_by_file; +- unsigned allow_textconv; +- unsigned textconv_set_via_cmdline; +- unsigned diff_from_contents; +- unsigned dirty_submodules; +- unsigned ignore_untracked_in_submodules; +- unsigned ignore_dirty_submodules; +- unsigned override_submodule_config; +- unsigned dirstat_by_line; +- unsigned funccontext; +- unsigned default_follow_renames; +- unsigned stat_with_summary; +- unsigned suppress_diff_headers; +- unsigned dual_color_diffed_diffs; ++ unsigned int recursive; ++ unsigned int tree_in_recursive; ++ unsigned int binary; ++ unsigned int text; ++ unsigned int full_index; ++ unsigned int silent_on_remove; ++ unsigned int find_copies_harder; ++ unsigned int follow_renames; ++ unsigned int rename_empty; ++ unsigned int has_changes; ++ unsigned int quick; ++ unsigned int no_index; ++ unsigned int allow_external; ++ unsigned int exit_with_status; ++ unsigned int reverse_diff; ++ unsigned int check_failed; ++ unsigned int relative_name; ++ unsigned int ignore_submodules; ++ unsigned int dirstat_cumulative; ++ unsigned int dirstat_by_file; ++ unsigned int allow_textconv; ++ unsigned int textconv_set_via_cmdline; ++ unsigned int diff_from_contents; ++ unsigned int dirty_submodules; ++ unsigned int ignore_untracked_in_submodules; ++ unsigned int ignore_dirty_submodules; ++ unsigned int override_submodule_config; ++ unsigned int dirstat_by_line; ++ unsigned int funccontext; ++ unsigned int default_follow_renames; ++ unsigned int stat_with_summary; ++ unsigned int suppress_diff_headers; ++ unsigned int dual_color_diffed_diffs; + }; + + static inline void diff_flags_or(struct diff_flags *a, +@@ -151,7 +151,7 @@ struct diff_options { + int skip_stat_unmatch; + int line_termination; + int output_format; +- unsigned pickaxe_opts; ++ unsigned int pickaxe_opts; + int rename_score; + int rename_limit; + int needed_rename_limit; +@@ -169,7 +169,7 @@ struct diff_options { + const char *prefix; + int prefix_length; + const char *stat_sep; +- long xdl_opts; ++ int xdl_opts; + + /* see Documentation/diff-options.txt */ + char **anchors; diff --git a/0002-diff-parseopt-restore-U-no-argument-behavior.patch b/0002-diff-parseopt-restore-U-no-argument-behavior.patch new file mode 100644 index 0000000..b29c262 --- /dev/null +++ b/0002-diff-parseopt-restore-U-no-argument-behavior.patch @@ -0,0 +1,183 @@ +From 23f586f2524569218de43c10cd41c3d271198a9e Mon Sep 17 00:00:00 2001 +From: =?UTF-8?q?Nguy=E1=BB=85n=20Th=C3=A1i=20Ng=E1=BB=8Dc=20Duy?= + +Date: Fri, 24 May 2019 16:24:41 +0700 +Subject: [PATCH 2/3] diff-parseopt: restore -U (no argument) behavior +MIME-Version: 1.0 +Content-Type: text/plain; charset=UTF-8 +Content-Transfer-Encoding: 8bit + +Before d473e2e0e8 (diff.c: convert -U|--unified, 2019-01-27), -U and +--unified are implemented with a custom parser opt_arg() in diff.c. I +didn't check this code carefully and not realize that it's the +equivalent of PARSE_OPT_NONEG | PARSE_OPT_OPTARG. + +In other words, if -U is specified without any argument, the option +should be accepted, and the default value should be used. Without +PARSE_OPT_OPTARG, parse_options() will reject this case and cause a +regression. + +Reported-by: Bryan Turner +Signed-off-by: Nguyễn Thái Ngọc Duy +--- + diff.c | 10 +++++---- + t/t4013-diff-various.sh | 2 ++ + t/t4013/diff.diff_-U1_initial..side | 29 ++++++++++++++++++++++++++ + t/t4013/diff.diff_-U2_initial..side | 31 ++++++++++++++++++++++++++++ + t/t4013/diff.diff_-U_initial..side | 32 +++++++++++++++++++++++++++++ + 5 files changed, 100 insertions(+), 4 deletions(-) + create mode 100644 t/t4013/diff.diff_-U1_initial..side + create mode 100644 t/t4013/diff.diff_-U2_initial..side + create mode 100644 t/t4013/diff.diff_-U_initial..side + +diff --git a/diff.c b/diff.c +index 4d3cf83a27..80ddc11671 100644 +--- a/diff.c ++++ b/diff.c +@@ -5211,9 +5211,11 @@ static int diff_opt_unified(const struct option *opt, + + BUG_ON_OPT_NEG(unset); + +- options->context = strtol(arg, &s, 10); +- if (*s) +- return error(_("%s expects a numerical value"), "--unified"); ++ if (arg) { ++ options->context = strtol(arg, &s, 10); ++ if (*s) ++ return error(_("%s expects a numerical value"), "--unified"); ++ } + enable_patch_output(&options->output_format); + + return 0; +@@ -5272,7 +5274,7 @@ static void prep_parse_options(struct diff_options *options) + DIFF_FORMAT_PATCH, DIFF_FORMAT_NO_OUTPUT), + OPT_CALLBACK_F('U', "unified", options, N_(""), + N_("generate diffs with lines context"), +- PARSE_OPT_NONEG, diff_opt_unified), ++ PARSE_OPT_NONEG | PARSE_OPT_OPTARG, diff_opt_unified), + OPT_BOOL('W', "function-context", &options->flags.funccontext, + N_("generate diffs with lines context")), + OPT_BIT_F(0, "raw", &options->output_format, +diff --git a/t/t4013-diff-various.sh b/t/t4013-diff-various.sh +index 9f8f0e84ad..a9054d2db1 100755 +--- a/t/t4013-diff-various.sh ++++ b/t/t4013-diff-various.sh +@@ -338,6 +338,8 @@ format-patch --inline --stdout initial..master^^ + format-patch --stdout --cover-letter -n initial..master^ + + diff --abbrev initial..side ++diff -U initial..side ++diff -U1 initial..side + diff -r initial..side + diff --stat initial..side + diff -r --stat initial..side +diff --git a/t/t4013/diff.diff_-U1_initial..side b/t/t4013/diff.diff_-U1_initial..side +new file mode 100644 +index 0000000000..b69f8f048a +--- /dev/null ++++ b/t/t4013/diff.diff_-U1_initial..side +@@ -0,0 +1,29 @@ ++$ git diff -U1 initial..side ++diff --git a/dir/sub b/dir/sub ++index 35d242b..7289e35 100644 ++--- a/dir/sub +++++ b/dir/sub ++@@ -2 +2,3 @@ A ++ B +++1 +++2 ++diff --git a/file0 b/file0 ++index 01e79c3..f4615da 100644 ++--- a/file0 +++++ b/file0 ++@@ -3 +3,4 @@ ++ 3 +++A +++B +++C ++diff --git a/file3 b/file3 ++new file mode 100644 ++index 0000000..7289e35 ++--- /dev/null +++++ b/file3 ++@@ -0,0 +1,4 @@ +++A +++B +++1 +++2 ++$ +diff --git a/t/t4013/diff.diff_-U2_initial..side b/t/t4013/diff.diff_-U2_initial..side +new file mode 100644 +index 0000000000..8ffe04f203 +--- /dev/null ++++ b/t/t4013/diff.diff_-U2_initial..side +@@ -0,0 +1,31 @@ ++$ git diff -U2 initial..side ++diff --git a/dir/sub b/dir/sub ++index 35d242b..7289e35 100644 ++--- a/dir/sub +++++ b/dir/sub ++@@ -1,2 +1,4 @@ ++ A ++ B +++1 +++2 ++diff --git a/file0 b/file0 ++index 01e79c3..f4615da 100644 ++--- a/file0 +++++ b/file0 ++@@ -2,2 +2,5 @@ ++ 2 ++ 3 +++A +++B +++C ++diff --git a/file3 b/file3 ++new file mode 100644 ++index 0000000..7289e35 ++--- /dev/null +++++ b/file3 ++@@ -0,0 +1,4 @@ +++A +++B +++1 +++2 ++$ +diff --git a/t/t4013/diff.diff_-U_initial..side b/t/t4013/diff.diff_-U_initial..side +new file mode 100644 +index 0000000000..c66c0dd5c6 +--- /dev/null ++++ b/t/t4013/diff.diff_-U_initial..side +@@ -0,0 +1,32 @@ ++$ git diff -U initial..side ++diff --git a/dir/sub b/dir/sub ++index 35d242b..7289e35 100644 ++--- a/dir/sub +++++ b/dir/sub ++@@ -1,2 +1,4 @@ ++ A ++ B +++1 +++2 ++diff --git a/file0 b/file0 ++index 01e79c3..f4615da 100644 ++--- a/file0 +++++ b/file0 ++@@ -1,3 +1,6 @@ ++ 1 ++ 2 ++ 3 +++A +++B +++C ++diff --git a/file3 b/file3 ++new file mode 100644 ++index 0000000..7289e35 ++--- /dev/null +++++ b/file3 ++@@ -0,0 +1,4 @@ +++A +++B +++1 +++2 ++$ diff --git a/0003-parse-options-check-empty-value-in-OPT_INTEGER-and-O.patch b/0003-parse-options-check-empty-value-in-OPT_INTEGER-and-O.patch new file mode 100644 index 0000000..f9ecde5 --- /dev/null +++ b/0003-parse-options-check-empty-value-in-OPT_INTEGER-and-O.patch @@ -0,0 +1,63 @@ +From 9709e02ba78d2408269fc81bd27691714ed26e26 Mon Sep 17 00:00:00 2001 +From: =?UTF-8?q?Nguy=E1=BB=85n=20Th=C3=A1i=20Ng=E1=BB=8Dc=20Duy?= + +Date: Fri, 24 May 2019 16:24:42 +0700 +Subject: [PATCH 3/3] parse-options: check empty value in OPT_INTEGER and + OPT_ABBREV +MIME-Version: 1.0 +Content-Type: text/plain; charset=UTF-8 +Content-Transfer-Encoding: 8bit + +When parsing the argument for OPT_INTEGER and OPT_ABBREV, we check if we +can parse the entire argument to a number with "if (*s)". There is one +missing check: if "arg" is empty to begin with, we fail to notice. + +This could happen with long option by writing like + + git diff --inter-hunk-context= blah blah + +Before 16ed6c97cc (diff-parseopt: convert --inter-hunk-context, +2019-03-24), --inter-hunk-context is handled by a custom parser +opt_arg() and does detect this correctly. + +This restores the bahvior for --inter-hunk-context and make sure all +other integer options are handled the same (sane) way. For OPT_ABBREV +this is new behavior. But it makes it consistent with the rest. + +PS. OPT_MAGNITUDE has similar code but git_parse_ulong() does detect +empty "arg". So it's good to go. + +Signed-off-by: Nguyễn Thái Ngọc Duy +--- + parse-options-cb.c | 3 +++ + parse-options.c | 3 +++ + 2 files changed, 6 insertions(+) + +diff --git a/parse-options-cb.c b/parse-options-cb.c +index 4b95d04a37..a3de795c58 100644 +--- a/parse-options-cb.c ++++ b/parse-options-cb.c +@@ -16,6 +16,9 @@ int parse_opt_abbrev_cb(const struct option *opt, const char *arg, int unset) + if (!arg) { + v = unset ? 0 : DEFAULT_ABBREV; + } else { ++ if (!*arg) ++ return error(_("option `%s' expects a numerical value"), ++ opt->long_name); + v = strtol(arg, (char **)&arg, 10); + if (*arg) + return error(_("option `%s' expects a numerical value"), +diff --git a/parse-options.c b/parse-options.c +index 987e27cb91..87b26a1d92 100644 +--- a/parse-options.c ++++ b/parse-options.c +@@ -195,6 +195,9 @@ static enum parse_opt_result get_value(struct parse_opt_ctx_t *p, + } + if (get_arg(p, opt, flags, &arg)) + return -1; ++ if (!*arg) ++ return error(_("%s expects a numerical value"), ++ optname(opt, flags)); + *(int *)opt->value = strtol(arg, (char **)&s, 10); + if (*s) + return error(_("%s expects a numerical value"), diff --git a/git.spec b/git.spec index 4f7485f..aea3e25 100644 --- a/git.spec +++ b/git.spec @@ -88,7 +88,7 @@ Name: git Version: 2.22.0 -Release: 0.1%{?rcrev}%{?dist} +Release: 0.2%{?rcrev}%{?dist} Summary: Fast Version Control System License: GPLv2 URL: https://git-scm.com/ @@ -120,6 +120,11 @@ Source99: print-failed-test-output # https://bugzilla.redhat.com/490602 Patch0: git-cvsimport-Ignore-cvsps-2.2b1-Branches-output.patch +# https://public-inbox.org/git/20190524092442.701-1-pclouds@gmail.com/T/ +Patch1: 0001-diff-parseopt-correct-variable-types-that-are-used-b.patch +Patch2: 0002-diff-parseopt-restore-U-no-argument-behavior.patch +Patch3: 0003-parse-options-check-empty-value-in-OPT_INTEGER-and-O.patch + %if %{with docs} # pod2man is needed to build Git.3pm BuildRequires: %{_bindir}/pod2man @@ -956,6 +961,9 @@ rmdir --ignore-fail-on-non-empty "$testdir" %{?with_docs:%{_pkgdocdir}/git-svn.html} %changelog +* Fri May 24 2019 Todd Zullinger - 2.22.0-0.2.rc1 +- Apply upstream fixes for diff-parseopt issues on s390x + * Sun May 19 2019 Todd Zullinger - 2.22.0-0.1.rc1 - Update to 2.22.0-rc1