Apply upstream fixes for diff-parseopt issues on s390x
References: https://public-inbox.org/git/20190523150416.GL3654@pobox.com/#t https://public-inbox.org/git/20190524092442.701-1-pclouds@gmail.com/T/
This commit is contained in:
Todd Zullinger
parent
554467c649
commit
9524a99a05
128
0001-diff-parseopt-correct-variable-types-that-are-used-b.patch
Normal file
128
0001-diff-parseopt-correct-variable-types-that-are-used-b.patch
Normal file
@ -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?=
|
||||
<pclouds@gmail.com>
|
||||
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 <tmz@pobox.com>
|
||||
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
|
||||
---
|
||||
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;
|
||||
183
0002-diff-parseopt-restore-U-no-argument-behavior.patch
Normal file
183
0002-diff-parseopt-restore-U-no-argument-behavior.patch
Normal file
@ -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?=
|
||||
<pclouds@gmail.com>
|
||||
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 <bturner@atlassian.com>
|
||||
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
|
||||
---
|
||||
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>"),
|
||||
N_("generate diffs with <n> 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 <n> 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
|
||||
+$
|
||||
@ -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?=
|
||||
<pclouds@gmail.com>
|
||||
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 <pclouds@gmail.com>
|
||||
---
|
||||
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"),
|
||||
10
git.spec
10
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 <tmz@pobox.com> - 2.22.0-0.2.rc1
|
||||
- Apply upstream fixes for diff-parseopt issues on s390x
|
||||
|
||||
* Sun May 19 2019 Todd Zullinger <tmz@pobox.com> - 2.22.0-0.1.rc1
|
||||
- Update to 2.22.0-rc1
|
||||
|
||||
|
||||
Loading…
Reference in New Issue
Block a user