[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[bug-diffutils] Re: Bug#577832: diffutils: [REGRESSION] newline is added
From: |
Paul Eggert |
Subject: |
[bug-diffutils] Re: Bug#577832: diffutils: [REGRESSION] newline is added to a line that has no newline if the line is in context (fwd) |
Date: |
Fri, 16 Apr 2010 22:32:56 -0700 |
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/23.1 (gnu/linux) |
Jim, thanks for fixing that bug. I did notice a couple of problems.
First, the newly added test cases did not detect the bug for me (the
old, buggy "diff" passed the new test cases). Second, there's a
slightly simpler fix that decouples the "bucket = &buckets[-1];" part
from the "linbuf[line]--;" part, while avoiding the need for the new
missing_newline_fixup variable.
I installed the following followup patch into the savannah repository
for diffutils. This patch also adds some comments to try to make this
stuff a bit clearer. I hope you don't mind my changing "--linbuf[line]"
to "linbuf[line]--", as that section of code uses a style that prefers
post- to pre-decrement.
-----
Followon improvements for the fix for Debian bug 577832.
* src/io.c (find_and_hash_each_line): Omit the inserted newline in
a simpler way.
* tests/no-newline-at-eof: Fix the test case so that it rejects
the old, buggy behavior.
diff --git a/src/io.c b/src/io.c
index 11f9ee3..031be3d 100644
--- a/src/io.c
+++ b/src/io.c
@@ -220,7 +220,6 @@ find_and_hash_each_line (struct file_data *current)
bool same_length_diff_contents_compare_anyway =
diff_length_compare_anyway | ignore_case;
- bool missing_newline_fixup = false;
while (p < suffix_begin)
{
char const *ip = p;
@@ -377,15 +376,13 @@ find_and_hash_each_line (struct file_data *current)
&& current->missing_newline
&& ROBUST_OUTPUT_STYLE (output_style))
{
- missing_newline_fixup = true;
- /* This line is incomplete. If this is significant,
- put the line into buckets[-1]. */
+ /* The last line is incomplete and we do not silently
+ complete lines. If the line cannot compare equal to any
+ complete line, put it into buckets[-1] so that it can
+ compare equal only to the other file's incomplete line
+ (if one exists). */
if (ignore_white_space < IGNORE_SPACE_CHANGE)
bucket = &buckets[-1];
-
- /* Omit the inserted newline when computing linbuf later. */
- p--;
- bufend = suffix_begin = p;
}
for (i = *bucket; ; i = eqs[i].next)
@@ -474,11 +471,10 @@ find_and_hash_each_line (struct file_data *current)
if (p == bufend)
{
- /* If we've added a newline sentinel and did not adjust "bufend"
- above, then linbuf[line] is now pointing at the sentinel, yet
- should instead be pointing to the preceding byte. */
- if (!missing_newline_fixup && current->missing_newline)
- --linbuf[line];
+ /* If the last line is incomplete and we do not silently
+ complete lines, don't count its appended newline. */
+ if (current->missing_newline && ROBUST_OUTPUT_STYLE (output_style))
+ linbuf[line]--;
break;
}
diff --git a/tests/no-newline-at-eof b/tests/no-newline-at-eof
index c3694a1..092d1cd 100644
--- a/tests/no-newline-at-eof
+++ b/tests/no-newline-at-eof
@@ -6,24 +6,28 @@
: ${srcdir=.}
. "$srcdir/init.sh"; path_prepend_ ../src
-printf '\n1' > a || framework_failure_
-printf '\n0\n\n1' > b || framework_failure_
+printf '\n1\n2\n3' > a || framework_failure_
+printf '\n0\n\n1\n2\n3' > b || framework_failure_
cat <<EOF > exp || framework_failure_
-@@ -1,2 +1,4 @@
+@@ -1,4 +1,6 @@
+0
+
1
+ 2
+ 3
\ No newline at end of file
EOF
cat <<EOF > exp2 || framework_failure_
-@@ -1,2 +1,4 @@
+@@ -1,4 +1,6 @@
--1
+0
+
-+1
+ 1
+ 2
+-3
++3
\ No newline at end of file
EOF
@@ -32,7 +36,7 @@ fail=0
# So we don't have to record trailing blanks in expected output above.
opt=--suppress-blank-empty
-diff $opt -U2 a b > out 2> err
+diff $opt -u a b > out 2> err
test $? = 1 || fail=1
sed -n '/^@@/,$p' out > k && mv k out || fail=1