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

[Kai YU]

OK. Since I found the letter in bug-diffutils is kind of messy, I rephrase our findings as below:

    We isolated the failure-inducing change introduced by diffutils-2.9:
diff -u ./diffutils-2.8.1/src/io.c ./diffutils-2.9/src/io.c

--- diffutils-2.8.1/src/io.c
+++ diffutils-2.9/src/io.c
@@ -664,2 +650,2 @@
-      for (; p0 != beg0; p0--, p1--)
-        if (*p0 != *p1)
+     while (p0 != beg0)
+       if (*--p0 != *--p1)
          {

            /* Point at the first char of the matching suffix.  */
+           ++p0, ++p1;
            beg0 = p0;
            break;
          }

    This change causes the pointer 'p0' wrongly points to somewhere different as it does in 2.8.after the loop, and later defines the variable filevec[0].suffix_begin, which leads to a failure in 2.9.

in diffutils-2.9/src/io.c:
static void
find_identical_ends (struct file_data filevec[])
{
...
  if (! ROBUST_OUTPUT_STYLE (output_style)
      || filevec[0].missing_newline == filevec[1].missing_newline)

    {
      ...
      while (p0 != beg0)
            if (*--p0 != *--p1)
             {
               ...
             }
      ...
    }
  /* Record the suffix.  */
  filevec[0].suffix_begin = p0;

  filevec[1].suffix_begin = p1;
...
}

We also notice that in 3.0 this bug has been fixed by adding the following code:

diff -u ./diffutils-2.9/src/io.c ./diffutils-3.0/src/io.c
--- diffutils-2.9/src/io.c

+++ diffutils-3.0/src/io.c
@@ -471,7 +470,13 @@
       linbuf[line] = p;

        if (p == bufend)
 -       break;
 +       {
 +         /* 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;
 +       }

        if (context <= i && no_diff_means_no_output)

         break;

      This repair does fix the bug. However,  this fix tries to adjust the final linbuf[line] setting rather than corrects the error cause. Instead, we suggest withdraw the change introduced in 2.9 may be a better choice. Besides, the value of filevec[0].suffix_begin in 3.0 is not equal to that in 2.8.1, which may cause potential bugs in the future.

2010/10/7 Jim Meyering <address@hidden>

Kai YU wrote:
> Dear Jim:

Thank you for taking the time to analyze diffutils bugs.
Please send any such mail to the bug-reporting address
you see in diff --help output: address@hidden

>        I and my colleague Xiangyu Zhang and Jin Chen recently examined Bug#
> 577832 in our study. Since this bug is a regression from diffutils-2.8.1, we
> isolated the failure-inducing change automatically:
>
>     diff -u ./diffutils-2.8.1/src/io.c ./diffutils-2.9/src/io.c
>     --- diffutils-2.8.1/src/io.c
>     +++ diffutils-2.9/src/io.c
>     @@ -664,2 +650,2 @@
>     -      for (; p0 != beg0; p0--, p1--)
>     -        if (*p0 != *p1)
>     +     while (p0 != beg0)
>     +       if (*--p0 != *--p1)
>               {
>                 /* Point at the first char of the matching suffix.  */
>     +           ++p0, ++p1;
>                 beg0 = p0;
>                 break;
>               }
>
>       This change causes the pointer 'p0' wrongly points to somewhere different
> as it does in 2.8.after the loop, and later defines the variable filevec
> [0].suffix_begin, which leads to a failure in 2.9.
>
>     in diffutils-2.9/src/io.c:
>
>     static void
>     find_identical_ends (struct file_data filevec[])
>     {
>     ...
>       if (! ROBUST_OUTPUT_STYLE (output_style)
>           || filevec[0].missing_newline == filevec[1].missing_newline)
>         {
>           ...
>           while (p0 != beg0)
>                 if (*--p0 != *--p1)
>                  {
>                    ...
>                  }
>           ...
>         }
>       /* Record the suffix.  */
>       filevec[0].suffix_begin = p0;
>       filevec[1].suffix_begin = p1;
>     ...
>     }
>
>        We also notice that in 3.0 this bug has been fixed by adding the
> following code:
>
>     diff -u ./diffutils-2.9/src/io.c ./diffutils-3.0/src/io.c
>     --- diffutils-2.9/src/io.c
>     +++ diffutils-3.0/src/io.c
>     @@ -471,7 +470,13 @@
>
>            linbuf[line] = p;
>
>            if (p == bufend)
>     -       break;
>     +       {
>     +         /* 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;
>     +       }
>
>            if (context <= i && no_diff_means_no_output)
>             break;
>
>        This repair does fix the bug. However,  this fix tries to adjust the
> final linbuf[line] setting rather than corrects the error cause.
> Instead, we suggest withdraw the change introduced in 2.9 may be a better
> choice. Besides, the value of filevec[0].suffix_begin in 3.0 is not equal to
> that in 2.8.1, which may cause potential bugs in the future.
>
>        Looking forward to your reply.
> ---
> Kai YU