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>
Thomas Gummerer committed
Oct 8, 2019 at 18:38 UTC
2b6a9b13ca4a687aeb0cad5f32e49711b3e67aaa
3 files changed
+92
-22
apply.c
+22
-21
@@ -1361,11 +1361,32 @@ int parse_git_diff_header(struct strbuf *root,
1361
if (check_header_line(*linenr, patch))
1362
return -1;
1363
if (res > 0)
1364
- return offset;
1364
+ goto done;
1365
break;
1366
}
1367
}
1368
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;
1390
return offset;
1391
}
1392
@@ -1546,26 +1567,6 @@ static int find_header(struct apply_state *state,
1567
return -128;
1568
if (git_hdr_len <= len)
1569
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;
1570
*hdrsize = git_hdr_len;
1571
return offset;
1572
}
t/t3206-range-diff.sh
+40
@@ -226,6 +226,46 @@ test_expect_success 'renamed file' '
226
test_cmp expected actual
227
'
228
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
+
269
test_expect_success 'file added and later removed' '
270
git range-diff --no-color --submodule=log topic...added-removed >actual &&
271
sed s/Z/\ /g >expected <<-EOF &&
t/t3206/history.export
+30
-1
@@ -55,7 +55,7 @@ A
55
19
56
20
57
58
-commit refs/heads/topic
58
+commit refs/heads/mode-only-change
59
mark :4
60
author Thomas Rast <trast@inf.ethz.ch> 1374485014 +0200
61
committer Thomas Rast <trast@inf.ethz.ch> 1374485014 +0200
@@ -678,3 +678,32 @@ s/12/B/
678
from :55
679
M 100644 :9 renamed-file
680
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
+