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

[Jim Meyering]

Paul Eggert wrote:
> 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.

Thanks, Paul.
Decrementing in only one place is definitely an improvement.

> * tests/no-newline-at-eof: Fix the test case so that it rejects
> the old, buggy behavior.

I'm glad you noticed that.
I went back over my notes and found the transcription error.
The original, smaller test was fine, but only with -U1, not the -U2 I used.

    $ ./diff -U1 <(printf '\n1') <(printf '\n0\n\n1')

Here's an incremental over my original:

$ g diff HEAD~2 tests/no-newline-at-eof
diff --git a/tests/no-newline-at-eof b/tests/no-newline-at-eof
index c3694a1..e5dc89d 100644
--- a/tests/no-newline-at-eof
+++ b/tests/no-newline-at-eof
@@ -32,7 +32,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 -U1 a b > out 2> err
 test $? = 1 || fail=1

 sed -n '/^@@/,$p' out > k && mv k out || fail=1
@@ -43,7 +43,7 @@ compare err /dev/null || fail=1
 # Repeat, but with a newline at the end of "a".
 echo >> a

-diff $opt -U2 a b > out 2> err
+diff $opt -U1 a b > out 2> err
 test $? = 1 || fail=1

 sed -n '/^@@/,$p' out > k && mv k out || fail=1

Here's what I've pushed:

>From 4014afece0ad0bde3628d81f865a7fcd28ce05be Mon Sep 17 00:00:00 2001
From: Jim Meyering <address@hidden>
Date: Sat, 17 Apr 2010 08:21:32 +0200
Subject: [PATCH] tests: use original no-newline-at-eof test, but with -U1, not 
-U2

* tests/no-newline-at-eof: Revert to the smaller test, but with
-U1 rather than -U2, since that actually triggers the bug.
---
 tests/no-newline-at-eof |   20 ++++++++------------
 1 files changed, 8 insertions(+), 12 deletions(-)

diff --git a/tests/no-newline-at-eof b/tests/no-newline-at-eof
index 092d1cd..e5dc89d 100644
--- a/tests/no-newline-at-eof
+++ b/tests/no-newline-at-eof
@@ -6,28 +6,24 @@
 : ${srcdir=.}
 . "$srcdir/init.sh"; path_prepend_ ../src

-printf '\n1\n2\n3'      > a || framework_failure_
-printf '\n0\n\n1\n2\n3' > b || framework_failure_
+printf '\n1'      > a || framework_failure_
+printf '\n0\n\n1' > b || framework_failure_
 cat <<EOF > exp || framework_failure_
-@@ -1,4 +1,6 @@
+@@ -1,2 +1,4 @@

 +0
 +
  1
- 2
- 3
 \ No newline at end of file
 EOF

 cat <<EOF > exp2 || framework_failure_
-@@ -1,4 +1,6 @@
+@@ -1,2 +1,4 @@

+-1
 +0
 +
- 1
- 2
--3
-+3
++1
 \ No newline at end of file
 EOF

@@ -36,7 +32,7 @@ fail=0
 # So we don't have to record trailing blanks in expected output above.
 opt=--suppress-blank-empty

-diff $opt -u a b > out 2> err
+diff $opt -U1 a b > out 2> err
 test $? = 1 || fail=1

 sed -n '/^@@/,$p' out > k && mv k out || fail=1
@@ -47,7 +43,7 @@ compare err /dev/null || fail=1
 # Repeat, but with a newline at the end of "a".
 echo >> a

-diff $opt -U2 a b > out 2> err
+diff $opt -U1 a b > out 2> err
 test $? = 1 || fail=1

 sed -n '/^@@/,$p' out > k && mv k out || fail=1
--
1.7.1.rc1.269.ga27c7