Skip to content

Commit 757e75c

Browse files
jerry-skydiogitster
authored andcommitted
patch-id: fix scan_hunk_header on diffs with 1 line of before/after
Normally diffs will contain a hunk header of the format "@@ -2,2 +2,15 @@ code". However when there is only 1 line of change, the unified diff format allows for the second comma separated value to be omitted in either before or after line counts. This can produce hunk headers that look like "@@ -2 +2,18 @@ code" or "@@ -2,2 +2 @@ code". As a result, scan_hunk_header mistakenly returns the line number as line count, which then results in unpredictable parsing errors with the rest of the patch, including giving multiple lines of output for a single commit. Fix by explicitly setting line count to 1 when there is no comma, and add a test. apply.c contains this same logic except it is correct. A worthwhile future project might be to unify these two diff parsers so they both benefit from fixes. Signed-off-by: Jerry Zhang <[email protected]> Signed-off-by: Junio C Hamano <[email protected]>
1 parent 56fa5ac commit 757e75c

File tree

2 files changed

+37
-3
lines changed

2 files changed

+37
-3
lines changed

builtin/patch-id.c

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -32,22 +32,27 @@ static int scan_hunk_header(const char *p, int *p_before, int *p_after)
3232
n = strspn(q, digits);
3333
if (q[n] == ',') {
3434
q += n + 1;
35+
*p_before = atoi(q);
3536
n = strspn(q, digits);
37+
} else {
38+
*p_before = 1;
3639
}
40+
3741
if (n == 0 || q[n] != ' ' || q[n+1] != '+')
3842
return 0;
3943

4044
r = q + n + 2;
4145
n = strspn(r, digits);
4246
if (r[n] == ',') {
4347
r += n + 1;
48+
*p_after = atoi(r);
4449
n = strspn(r, digits);
50+
} else {
51+
*p_after = 1;
4552
}
4653
if (n == 0)
4754
return 0;
4855

49-
*p_before = atoi(q);
50-
*p_after = atoi(r);
5156
return 1;
5257
}
5358

t/t4204-patch-id.sh

Lines changed: 30 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,7 @@ calc_patch_id () {
3838
shift
3939
git patch-id "$@" >patch-id.output &&
4040
sed "s/ .*//" patch-id.output >patch-id_"$patch_name" &&
41-
test_line_count -gt 0 patch-id_"$patch_name"
41+
test_line_count -eq 1 patch-id_"$patch_name"
4242
}
4343

4444
get_top_diff () {
@@ -200,4 +200,33 @@ test_expect_success 'patch-id handles no-nl-at-eof markers' '
200200
calc_patch_id withnl <withnl &&
201201
test_cmp patch-id_nonl patch-id_withnl
202202
'
203+
204+
test_expect_success 'patch-id handles diffs with one line of before/after' '
205+
cat >diffu1 <<-\EOF &&
206+
diff --git a/bar b/bar
207+
index bdaf90f..31051f6 100644
208+
--- a/bar
209+
+++ b/bar
210+
@@ -2 +2,2 @@
211+
b
212+
+c
213+
diff --git a/car b/car
214+
index 00750ed..2ae5e34 100644
215+
--- a/car
216+
+++ b/car
217+
@@ -1 +1,2 @@
218+
3
219+
+d
220+
diff --git a/foo b/foo
221+
index e439850..7146eb8 100644
222+
--- a/foo
223+
+++ b/foo
224+
@@ -2 +2,2 @@
225+
a
226+
+e
227+
EOF
228+
calc_patch_id diffu1 <diffu1 &&
229+
test_config patchid.stable true &&
230+
calc_patch_id diffu1stable <diffu1
231+
'
203232
test_done

0 commit comments

Comments
 (0)