Skip to content

Commit 2344d47

Browse files
dschoJunio C Hamano
dscho
authored and
Junio C Hamano
committed
diff: fix 2 whitespace issues
When whitespace or whitespace change was ignored, the function xdl_recmatch() returned memcmp() style differences, which is wrong, since it should return 0 on non-match. Also, there were three horrible off-by-one bugs, even leading to wrong hashes in the whitespace special handling. The issue was noticed by Ray Lehtiniemi. For good measure, this commit adds a test. Signed-off-by: Johannes Schindelin <Johannes.Schindelin@gmx.de> Signed-off-by: Junio C Hamano <junkio@cox.net>
1 parent 854de5a commit 2344d47

File tree

2 files changed

+134
-17
lines changed

2 files changed

+134
-17
lines changed

‎t/t4015-diff-whitespace.sh‎

Lines changed: 122 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,122 @@
1+
#!/bin/sh
2+
#
3+
# Copyright (c) 2006 Johannes E. Schindelin
4+
#
5+
6+
test_description='Test special whitespace in diff engine.
7+
8+
'
9+
. ./test-lib.sh
10+
. ../diff-lib.sh
11+
12+
# Ray Lehtiniemi's example
13+
14+
cat << EOF > x
15+
do {
16+
nothing;
17+
} while (0);
18+
EOF
19+
20+
git-update-index --add x
21+
22+
cat << EOF > x
23+
do
24+
{
25+
nothing;
26+
}
27+
while (0);
28+
EOF
29+
30+
cat << EOF > expect
31+
diff --git a/x b/x
32+
index adf3937..6edc172 100644
33+
--- a/x
34+
+++ b/x
35+
@@ -1,3 +1,5 @@
36+
-do {
37+
+do
38+
+{
39+
nothing;
40+
-} while (0);
41+
+}
42+
+while (0);
43+
EOF
44+
45+
git-diff > out
46+
test_expect_success "Ray's example without options" 'diff -u expect out'
47+
48+
git-diff -w > out
49+
test_expect_success "Ray's example with -w" 'diff -u expect out'
50+
51+
git-diff -b > out
52+
test_expect_success "Ray's example with -b" 'diff -u expect out'
53+
54+
cat << EOF > x
55+
whitespace at beginning
56+
whitespace change
57+
whitespace in the middle
58+
whitespace at end
59+
unchanged line
60+
CR at end
61+
EOF
62+
63+
git-update-index x
64+
65+
cat << EOF > x
66+
whitespace at beginning
67+
whitespace change
68+
white space in the middle
69+
whitespace at end
70+
unchanged line
71+
CR at end
72+
EOF
73+
74+
cat << EOF > expect
75+
diff --git a/x b/x
76+
index d99af23..8b32fb5 100644
77+
--- a/x
78+
+++ b/x
79+
@@ -1,6 +1,6 @@
80+
-whitespace at beginning
81+
-whitespace change
82+
-whitespace in the middle
83+
-whitespace at end
84+
+ whitespace at beginning
85+
+whitespace change
86+
+white space in the middle
87+
+whitespace at end
88+
unchanged line
89+
-CR at end
90+
+CR at end
91+
EOF
92+
git-diff > out
93+
test_expect_success 'another test, without options' 'diff -u expect out'
94+
95+
cat << EOF > expect
96+
diff --git a/x b/x
97+
index d99af23..8b32fb5 100644
98+
EOF
99+
git-diff -w > out
100+
test_expect_success 'another test, with -w' 'diff -u expect out'
101+
102+
cat << EOF > expect
103+
diff --git a/x b/x
104+
index d99af23..8b32fb5 100644
105+
--- a/x
106+
+++ b/x
107+
@@ -1,6 +1,6 @@
108+
-whitespace at beginning
109+
+ whitespace at beginning
110+
whitespace change
111+
-whitespace in the middle
112+
-whitespace at end
113+
+white space in the middle
114+
+whitespace at end
115+
unchanged line
116+
-CR at end
117+
+CR at end
118+
EOF
119+
git-diff -b > out
120+
test_expect_success 'another test, with -b' 'diff -u expect out'
121+
122+
test_done

‎xdiff/xutils.c‎

Lines changed: 12 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -191,36 +191,30 @@ int xdl_recmatch(const char *l1, long s1, const char *l2, long s2, long flags)
191191
int i1, i2;
192192

193193
if (flags & XDF_IGNORE_WHITESPACE) {
194-
for (i1 = i2 = 0; i1 < s1 && i2 < s2; i1++, i2++) {
194+
for (i1 = i2 = 0; i1 < s1 && i2 < s2; ) {
195195
if (isspace(l1[i1]))
196196
while (isspace(l1[i1]) && i1 < s1)
197197
i1++;
198-
else if (isspace(l2[i2]))
198+
if (isspace(l2[i2]))
199199
while (isspace(l2[i2]) && i2 < s2)
200200
i2++;
201-
else if (l1[i1] != l2[i2])
202-
return l2[i2] - l1[i1];
201+
if (i1 < s1 && i2 < s2 && l1[i1++] != l2[i2++])
202+
return 0;
203203
}
204-
if (i1 >= s1)
205-
return 1;
206-
else if (i2 >= s2)
207-
return -1;
204+
return (i1 >= s1 && i2 >= s2);
208205
} else if (flags & XDF_IGNORE_WHITESPACE_CHANGE) {
209-
for (i1 = i2 = 0; i1 < s1 && i2 < s2; i1++, i2++) {
206+
for (i1 = i2 = 0; i1 < s1 && i2 < s2; ) {
210207
if (isspace(l1[i1])) {
211208
if (!isspace(l2[i2]))
212-
return -1;
209+
return 0;
213210
while (isspace(l1[i1]) && i1 < s1)
214211
i1++;
215212
while (isspace(l2[i2]) && i2 < s2)
216213
i2++;
217-
} else if (l1[i1] != l2[i2])
218-
return l2[i2] - l1[i1];
214+
} else if (l1[i1++] != l2[i2++])
215+
return 0;
219216
}
220-
if (i1 >= s1)
221-
return 1;
222-
else if (i2 >= s2)
223-
return -1;
217+
return (i1 >= s1 && i2 >= s2);
224218
} else
225219
return s1 == s2 && !memcmp(l1, l2, s1);
226220

@@ -233,7 +227,8 @@ unsigned long xdl_hash_record(char const **data, char const *top, long flags) {
233227

234228
for (; ptr < top && *ptr != '\n'; ptr++) {
235229
if (isspace(*ptr) && (flags & XDF_WHITESPACE_FLAGS)) {
236-
while (ptr < top && isspace(*ptr) && ptr[1] != '\n')
230+
while (ptr + 1 < top && isspace(ptr[1])
231+
&& ptr[1] != '\n')
237232
ptr++;
238233
if (flags & XDF_IGNORE_WHITESPACE_CHANGE) {
239234
ha += (ha << 5);

0 commit comments

Comments
 (0)