@matheusbsbs / matheus-bagliano / commits / fa5ba2c1dd

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>

Jeff King committed Oct 12, 2017 at 20:18 UTC fa5ba2c1dd0ce1bd060f423e7b1eb39d06fcd2cd

2 files changed +24 -13

diff.c

+15 -13

@@ -720,20 +720,22 @@ static int next_byte(const char **cp, const char **endp,

720 if (*cp > *endp)

721 return -1;

722

723 - if (DIFF_XDL_TST(diffopt, IGNORE_WHITESPACE_CHANGE)) {

724 - while (*cp < *endp && isspace(**cp))

725 - (*cp)++;

726 - /*

727 - * After skipping a couple of whitespaces, we still have to

728 - * account for one space.

729 - */

730 - return (int)' ';

731 - }

723 + if (isspace(**cp)) {

724 + if (DIFF_XDL_TST(diffopt, IGNORE_WHITESPACE_CHANGE)) {

725 + while (*cp < *endp && isspace(**cp))

726 + (*cp)++;

727 + /*

728 + * After skipping a couple of whitespaces,

729 + * we still have to account for one space.

730 + */

731 + return (int)' ';

732 + }

733

733 - if (DIFF_XDL_TST(diffopt, IGNORE_WHITESPACE)) {

734 - while (*cp < *endp && isspace(**cp))

735 - (*cp)++;

736 - /* return the first non-ws character via the usual below */

734 + if (DIFF_XDL_TST(diffopt, IGNORE_WHITESPACE)) {

735 + while (*cp < *endp && isspace(**cp))

736 + (*cp)++;

737 + /* return the first non-ws character via the usual below */

738 + }

739 }

740

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

t/t4015-diff-whitespace.sh

@@ -1406,4 +1406,13 @@ test_expect_success 'move detection with submodules' '

1406 test_cmp expect decoded_actual

1407 '

1408

1409 +test_expect_success 'move detection with whitespace changes' '

1410 + test_when_finished "git reset --hard" &&

1411 + test_seq 10 >test &&

1412 + git add test &&

1413 + sed s/3/42/ <test >test.tmp &&

1414 + mv test.tmp test &&

1415 + git -c diff.colormoved diff --ignore-space-change -- test

1416 +'

1417 +

1418 test_done