diff: fix infinite loop with --color-moved --ignore-space-change

The --color-moved code uses next_byte() to advance through
the blob contents. When the user has asked to ignore
whitespace changes, we try to collapse any whitespace change
down to a single space.

However, we enter the conditional block whenever we see the
IGNORE_WHITESPACE_CHANGE flag, even if the next byte isn't
whitespace.

This means that the combination of "--color-moved and
--ignore-space-change" was completely broken. Worse, because
we return from next_byte() without having advanced our
pointer, the function makes no forward progress in the
buffer and loops infinitely.

Fix this by entering the conditional only when we actually
see whitespace. We can apply this also to the
IGNORE_WHITESPACE change. That code path isn't buggy
(because it falls through to returning the next
non-whitespace byte), but it makes the logic more clear if
we only bother to look at whitespace flags after seeing that
the next byte is whitespace.

Reported-by: Orgad Shaneh <orgads@gmail.com>
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

diff --git a/diff.c b/diff.c
index 0c604726c06b07c2cd17bbdf05dc669ae92e5ae4..af9b9528861f4dd1ea901b5c9bc897e335cf2cb1 100644 (file)
--- a/diff.c
+++ b/diff.c
@@ -720,20 +720,22 @@ static int next_byte(const char **cp, const char **endp,
        if (*cp > *endp)
                return -1;
 
        if (*cp > *endp)
                return -1;
 

-       if (DIFF_XDL_TST(diffopt, IGNORE_WHITESPACE_CHANGE)) {
-               while (*cp < *endp && isspace(**cp))
-                       (*cp)++;
-               /*
-                * After skipping a couple of whitespaces, we still have to
-                * account for one space.
-                */
-               return (int)' ';
-       }
+       if (isspace(**cp)) {
+               if (DIFF_XDL_TST(diffopt, IGNORE_WHITESPACE_CHANGE)) {
+                       while (*cp < *endp && isspace(**cp))
+                               (*cp)++;
+                       /*
+                        * After skipping a couple of whitespaces,
+                        * we still have to account for one space.
+                        */
+                       return (int)' ';
+               }

 
 

-       if (DIFF_XDL_TST(diffopt, IGNORE_WHITESPACE)) {
-               while (*cp < *endp && isspace(**cp))
-                       (*cp)++;
-               /* return the first non-ws character via the usual below */
+               if (DIFF_XDL_TST(diffopt, IGNORE_WHITESPACE)) {
+                       while (*cp < *endp && isspace(**cp))
+                               (*cp)++;
+                       /* return the first non-ws character via the usual below */
+               }

        }
 
        retval = (unsigned char)(**cp);
        }
 
        retval = (unsigned char)(**cp);

diff --git a/t/t4015-diff-whitespace.sh b/t/t4015-diff-whitespace.sh
index c3b697411ab16a0dc21ce7579046765ccbeddfbd..66cad4d629bf19cfcba23c452a7d35b7322f8205 100755 (executable)
--- a/t/t4015-diff-whitespace.sh
+++ b/t/t4015-diff-whitespace.sh
@@ -1406,4 +1406,13 @@ test_expect_success 'move detection with submodules' '
        test_cmp expect decoded_actual
 '
 
        test_cmp expect decoded_actual
 '
 

+test_expect_success 'move detection with whitespace changes' '
+       test_when_finished "git reset --hard" &&
+       test_seq 10 >test &&
+       git add test &&
+       sed s/3/42/ <test >test.tmp &&
+       mv test.tmp test &&
+       git -c diff.colormoved diff --ignore-space-change -- test
+'
+

 test_done
 test_done