[bug-diffutils] Re: Bug#577832: diffutils: [REGRESSION] newline is added to a line that has no newline if the line is in context (fwd)

[Paul Eggert]

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