Skip to content

Commit 2b6a9b1

Browse files
tgummerergitster
tgummerer
authored and
gitster
committed
range-diff: don't segfault with mode-only changes
In ef283b3 ("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>
1 parent 499352c commit 2b6a9b1

File tree

3 files changed

+92
-22
lines changed

3 files changed

+92
-22
lines changed

‎apply.c‎

Lines changed: 22 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -1361,11 +1361,32 @@ int parse_git_diff_header(struct strbuf *root,
13611361
if (check_header_line(*linenr, patch))
13621362
return -1;
13631363
if (res > 0)
1364-
return offset;
1364+
goto done;
13651365
break;
13661366
}
13671367
}
13681368

1369+
done:
1370+
if (!patch->old_name && !patch->new_name) {
1371+
if (!patch->def_name) {
1372+
error(Q_("git diff header lacks filename information when removing "
1373+
"%d leading pathname component (line %d)",
1374+
"git diff header lacks filename information when removing "
1375+
"%d leading pathname components (line %d)",
1376+
parse_hdr_state.p_value),
1377+
parse_hdr_state.p_value, *linenr);
1378+
return -128;
1379+
}
1380+
patch->old_name = xstrdup(patch->def_name);
1381+
patch->new_name = xstrdup(patch->def_name);
1382+
}
1383+
if ((!patch->new_name && !patch->is_delete) ||
1384+
(!patch->old_name && !patch->is_new)) {
1385+
error(_("git diff header lacks filename information "
1386+
"(line %d)"), *linenr);
1387+
return -128;
1388+
}
1389+
patch->is_toplevel_relative = 1;
13691390
return offset;
13701391
}
13711392

@@ -1546,26 +1567,6 @@ static int find_header(struct apply_state *state,
15461567
return -128;
15471568
if (git_hdr_len <= len)
15481569
continue;
1549-
if (!patch->old_name && !patch->new_name) {
1550-
if (!patch->def_name) {
1551-
error(Q_("git diff header lacks filename information when removing "
1552-
"%d leading pathname component (line %d)",
1553-
"git diff header lacks filename information when removing "
1554-
"%d leading pathname components (line %d)",
1555-
state->p_value),
1556-
state->p_value, state->linenr);
1557-
return -128;
1558-
}
1559-
patch->old_name = xstrdup(patch->def_name);
1560-
patch->new_name = xstrdup(patch->def_name);
1561-
}
1562-
if ((!patch->new_name && !patch->is_delete) ||
1563-
(!patch->old_name && !patch->is_new)) {
1564-
error(_("git diff header lacks filename information "
1565-
"(line %d)"), state->linenr);
1566-
return -128;
1567-
}
1568-
patch->is_toplevel_relative = 1;
15691570
*hdrsize = git_hdr_len;
15701571
return offset;
15711572
}

‎t/t3206-range-diff.sh‎

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -226,6 +226,46 @@ test_expect_success 'renamed file' '
226226
test_cmp expected actual
227227
'
228228

229+
test_expect_success 'file with mode only change' '
230+
git range-diff --no-color --submodule=log topic...mode-only-change >actual &&
231+
sed s/Z/\ /g >expected <<-EOF &&
232+
1: fccce22 ! 1: 4d39cb3 s/4/A/
233+
@@ Metadata
234+
ZAuthor: Thomas Rast <trast@inf.ethz.ch>
235+
Z
236+
Z ## Commit message ##
237+
- s/4/A/
238+
+ s/4/A/ + add other-file
239+
Z
240+
Z ## file ##
241+
Z@@
242+
@@ file
243+
Z A
244+
Z 6
245+
Z 7
246+
+
247+
+ ## other-file (new) ##
248+
2: 147e64e ! 2: 26c107f s/11/B/
249+
@@ Metadata
250+
ZAuthor: Thomas Rast <trast@inf.ethz.ch>
251+
Z
252+
Z ## Commit message ##
253+
- s/11/B/
254+
+ s/11/B/ + mode change other-file
255+
Z
256+
Z ## file ##
257+
Z@@ file: A
258+
@@ file: A
259+
Z 12
260+
Z 13
261+
Z 14
262+
+
263+
+ ## other-file (mode change 100644 => 100755) ##
264+
3: a63e992 = 3: 4c1e0f5 s/12/B/
265+
EOF
266+
test_cmp expected actual
267+
'
268+
229269
test_expect_success 'file added and later removed' '
230270
git range-diff --no-color --submodule=log topic...added-removed >actual &&
231271
sed s/Z/\ /g >expected <<-EOF &&

‎t/t3206/history.export‎

Lines changed: 30 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -55,7 +55,7 @@ A
5555
19
5656
20
5757

58-
commit refs/heads/topic
58+
commit refs/heads/mode-only-change
5959
mark :4
6060
author Thomas Rast <trast@inf.ethz.ch> 1374485014 +0200
6161
committer Thomas Rast <trast@inf.ethz.ch> 1374485014 +0200
@@ -678,3 +678,32 @@ s/12/B/
678678
from :55
679679
M 100644 :9 renamed-file
680680

681+
commit refs/heads/mode-only-change
682+
mark :57
683+
author Thomas Rast <trast@inf.ethz.ch> 1374485024 +0200
684+
committer Thomas Gummerer <t.gummerer@gmail.com> 1570473767 +0100
685+
data 24
686+
s/4/A/ + add other-file
687+
from :4
688+
M 100644 :5 file
689+
M 100644 :49 other-file
690+
691+
commit refs/heads/mode-only-change
692+
mark :58
693+
author Thomas Rast <trast@inf.ethz.ch> 1374485036 +0200
694+
committer Thomas Gummerer <t.gummerer@gmail.com> 1570473768 +0100
695+
data 33
696+
s/11/B/ + mode change other-file
697+
from :57
698+
M 100644 :7 file
699+
M 100755 :49 other-file
700+
701+
commit refs/heads/mode-only-change
702+
mark :59
703+
author Thomas Rast <trast@inf.ethz.ch> 1374485044 +0200
704+
committer Thomas Gummerer <t.gummerer@gmail.com> 1570473768 +0100
705+
data 8
706+
s/12/B/
707+
from :58
708+
M 100644 :9 file
709+

0 commit comments

Comments
 (0)