range-diff: don't segfault with mode-only changes
In ef283b3699 ("apply: make parse_git_diff_header public", 2019-07-11)
the 'parse_git_diff_header' function was made public and useable by
callers outside of apply.c.
However it was missed that its (then) only caller, 'find_header' did
some error handling, and completing 'struct patch' appropriately.
range-diff then started using this function, and tried to handle this
appropriately itself, but fell short in some cases. This in turn
would lead to range-diff segfaulting when there are mode-only changes
in a range.
Move the error handling and completing of the struct into the
'parse_git_diff_header' function, so other callers can take advantage
of it. This fixes the segfault in 'git range-diff'.
Reported-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
Signed-off-by: Thomas Gummerer <t.gummerer@gmail.com>
Acked-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
diff --git a/apply.c b/apply.c
index cde9536..5f06a14 100644
--- a/apply.c
+++ b/apply.c
@@ -1361,11 +1361,32 @@ int parse_git_diff_header(struct strbuf *root,
if (check_header_line(*linenr, patch))
return -1;
if (res > 0)
- return offset;
+ goto done;
break;
}
}
+done:
+ if (!patch->old_name && !patch->new_name) {
+ if (!patch->def_name) {
+ error(Q_("git diff header lacks filename information when removing "
+ "%d leading pathname component (line %d)",
+ "git diff header lacks filename information when removing "
+ "%d leading pathname components (line %d)",
+ parse_hdr_state.p_value),
+ parse_hdr_state.p_value, *linenr);
+ return -128;
+ }
+ patch->old_name = xstrdup(patch->def_name);
+ patch->new_name = xstrdup(patch->def_name);
+ }
+ if ((!patch->new_name && !patch->is_delete) ||
+ (!patch->old_name && !patch->is_new)) {
+ error(_("git diff header lacks filename information "
+ "(line %d)"), *linenr);
+ return -128;
+ }
+ patch->is_toplevel_relative = 1;
return offset;
}
@@ -1546,26 +1567,6 @@ static int find_header(struct apply_state *state,
return -128;
if (git_hdr_len <= len)
continue;
- if (!patch->old_name && !patch->new_name) {
- if (!patch->def_name) {
- error(Q_("git diff header lacks filename information when removing "
- "%d leading pathname component (line %d)",
- "git diff header lacks filename information when removing "
- "%d leading pathname components (line %d)",
- state->p_value),
- state->p_value, state->linenr);
- return -128;
- }
- patch->old_name = xstrdup(patch->def_name);
- patch->new_name = xstrdup(patch->def_name);
- }
- if ((!patch->new_name && !patch->is_delete) ||
- (!patch->old_name && !patch->is_new)) {
- error(_("git diff header lacks filename information "
- "(line %d)"), state->linenr);
- return -128;
- }
- patch->is_toplevel_relative = 1;
*hdrsize = git_hdr_len;
return offset;
}
diff --git a/t/t3206-range-diff.sh b/t/t3206-range-diff.sh
index ec54865..5b87fea 100755
--- a/t/t3206-range-diff.sh
+++ b/t/t3206-range-diff.sh
@@ -226,6 +226,46 @@
test_cmp expected actual
'
+test_expect_success 'file with mode only change' '
+ git range-diff --no-color --submodule=log topic...mode-only-change >actual &&
+ sed s/Z/\ /g >expected <<-EOF &&
+ 1: fccce22 ! 1: 4d39cb3 s/4/A/
+ @@ Metadata
+ ZAuthor: Thomas Rast <trast@inf.ethz.ch>
+ Z
+ Z ## Commit message ##
+ - s/4/A/
+ + s/4/A/ + add other-file
+ Z
+ Z ## file ##
+ Z@@
+ @@ file
+ Z A
+ Z 6
+ Z 7
+ +
+ + ## other-file (new) ##
+ 2: 147e64e ! 2: 26c107f s/11/B/
+ @@ Metadata
+ ZAuthor: Thomas Rast <trast@inf.ethz.ch>
+ Z
+ Z ## Commit message ##
+ - s/11/B/
+ + s/11/B/ + mode change other-file
+ Z
+ Z ## file ##
+ Z@@ file: A
+ @@ file: A
+ Z 12
+ Z 13
+ Z 14
+ +
+ + ## other-file (mode change 100644 => 100755) ##
+ 3: a63e992 = 3: 4c1e0f5 s/12/B/
+ EOF
+ test_cmp expected actual
+'
+
test_expect_success 'file added and later removed' '
git range-diff --no-color --submodule=log topic...added-removed >actual &&
sed s/Z/\ /g >expected <<-EOF &&
diff --git a/t/t3206/history.export b/t/t3206/history.export
index 7bb3814..4c808e5 100644
--- a/t/t3206/history.export
+++ b/t/t3206/history.export
@@ -55,7 +55,7 @@
19
20
-commit refs/heads/topic
+commit refs/heads/mode-only-change
mark :4
author Thomas Rast <trast@inf.ethz.ch> 1374485014 +0200
committer Thomas Rast <trast@inf.ethz.ch> 1374485014 +0200
@@ -678,3 +678,32 @@
from :55
M 100644 :9 renamed-file
+commit refs/heads/mode-only-change
+mark :57
+author Thomas Rast <trast@inf.ethz.ch> 1374485024 +0200
+committer Thomas Gummerer <t.gummerer@gmail.com> 1570473767 +0100
+data 24
+s/4/A/ + add other-file
+from :4
+M 100644 :5 file
+M 100644 :49 other-file
+
+commit refs/heads/mode-only-change
+mark :58
+author Thomas Rast <trast@inf.ethz.ch> 1374485036 +0200
+committer Thomas Gummerer <t.gummerer@gmail.com> 1570473768 +0100
+data 33
+s/11/B/ + mode change other-file
+from :57
+M 100644 :7 file
+M 100755 :49 other-file
+
+commit refs/heads/mode-only-change
+mark :59
+author Thomas Rast <trast@inf.ethz.ch> 1374485044 +0200
+committer Thomas Gummerer <t.gummerer@gmail.com> 1570473768 +0100
+data 8
+s/12/B/
+from :58
+M 100644 :9 file
+
