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 <jerry@skydio.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
This commit is contained in:
Jerry Zhang 2022-02-01 20:19:45 -08:00 committed by Junio C Hamano
parent 56fa5ac39a
commit 757e75c81e
2 changed files with 37 additions and 3 deletions

View file

@ -32,8 +32,12 @@ static int scan_hunk_header(const char *p, int *p_before, int *p_after)
n = strspn(q, digits);
if (q[n] == ',') {
q += n + 1;
*p_before = atoi(q);
n = strspn(q, digits);
} else {
*p_before = 1;
}
if (n == 0 || q[n] != ' ' || q[n+1] != '+')
return 0;
@ -41,13 +45,14 @@ static int scan_hunk_header(const char *p, int *p_before, int *p_after)
n = strspn(r, digits);
if (r[n] == ',') {
r += n + 1;
*p_after = atoi(r);
n = strspn(r, digits);
} else {
*p_after = 1;
}
if (n == 0)
return 0;
*p_before = atoi(q);
*p_after = atoi(r);
return 1;
}

View file

@ -38,7 +38,7 @@ calc_patch_id () {
shift
git patch-id "$@" >patch-id.output &&
sed "s/ .*//" patch-id.output >patch-id_"$patch_name" &&
test_line_count -gt 0 patch-id_"$patch_name"
test_line_count -eq 1 patch-id_"$patch_name"
}
get_top_diff () {
@ -200,4 +200,33 @@ test_expect_success 'patch-id handles no-nl-at-eof markers' '
calc_patch_id withnl <withnl &&
test_cmp patch-id_nonl patch-id_withnl
'
test_expect_success 'patch-id handles diffs with one line of before/after' '
cat >diffu1 <<-\EOF &&
diff --git a/bar b/bar
index bdaf90f..31051f6 100644
--- a/bar
+++ b/bar
@@ -2 +2,2 @@
b
+c
diff --git a/car b/car
index 00750ed..2ae5e34 100644
--- a/car
+++ b/car
@@ -1 +1,2 @@
3
+d
diff --git a/foo b/foo
index e439850..7146eb8 100644
--- a/foo
+++ b/foo
@@ -2 +2,2 @@
a
+e
EOF
calc_patch_id diffu1 <diffu1 &&
test_config patchid.stable true &&
calc_patch_id diffu1stable <diffu1
'
test_done