News for hackers. Stuff that matters.

A comment on A 35-year-old bug in Patch found in efforts to restore 29-year-old BSD.

Re:A 35-year-old bug in Patch found in efforts to restore 29... by kazinator on Monday August 17, 2020 @02:47PM (#24190425)
I cannot reproduce this with an installation of GNU patch 2.7.6.

I could be doing something wrong or misunderstanding the bug.

Input files:

  $ cat patch-bug-test
  How
  now
  brown
  cow?
  Now
  is
  the
  time
  for
  all
  good
  men.

  $ cat patch-bug-test-2
  How
  now
  brown
  cow?
  Now
  is
  the
  time
  for
Context diff:

  $ diff -c patch-bug-test patch-bug-test-2
  *** patch-bug-test 2020-08-17 11:39:03.056723058 -0700
  --- patch-bug-test-2 2020-08-17 11:41:46.683095324 -0700
  ***************
  *** 7,12 ****
    the
    time
    for
  - all
  - good
  - men.
  --- 7,9 ----
Apply in reverse to copy of patch-bug-test-2:

  $ cp patch-bug-test-2 patch-bug-test-3
  $ diff -c patch-bug-test patch-bug-test-2 | patch -R patch-bug-test-3
  patching file patch-bug-test-3
The operation is successful and the reverse-patched is now identical to the original:

  $ diff patch-bug-test patch-bug-test-3 
  # no output
I did try it with different numbers of lines removed.

Looking at the code in the GNU version, the function is quite different. That block of code is found, with the comment intact, but there are differences. It looks like this in the GNU patch repository, as of this commit: http://git.savannah.gnu.org/cgit/patch.git/tree/src/pch.c?id...

     if (!chars_read) {
       if (repl_beginning && repl_could_be_missing) {
          repl_missing = true;
          goto hunk_done;
       }
       if (p_max - p_end < 4) {
         strcpy (buf, "  \n");  /* assume blank lines got chopped */
         chars_read = 3;
       } else {
         fatal ("unexpected end of file in patch");
       }
     }
The unpatched FreeBSD one referenced in the article:

   if (len == 0) {
     if (p_max - p_end < 4) {
       /* assume blank lines got chopped */
       strlcpy(buf, "  \n", buf_size);
     } else {
       if (repl_beginning && repl_could_be_missing) {
         repl_missing = true;
         goto hunk_done;
       }
       fatal("unexpected end of file in patch\n");
     }
   }
The order of the tests is reversed, which could make a difference (though obviously only in the case when repl_beginng and repl_could_be_missing are true and the goto is taken). In the oldest baseline that is in the GNU patch repo (2009-dated), it is already this way, so to find the commit which affected this code we would have to look to earlier GNU patch sources.
[ Parent | on HN ]
Re:A 35-year-old bug in Patch found in efforts to restore 29... by Someone on Monday August 17, 2020 @03:34PM (#24190961)
I don’t think GNU patch shares history with Larry Wall’s original. https://directory.fsf.org/wiki/Patch seems to say so: ”GNU version of Larry Wall's program that takes "diff's" output and applies it to an original file to generate a modified version of that file”

I tried to verify by looking at their git repo, but that stops at “Import of patch-2.1.tar.gz” (https://git.savannah.gnu.org/cgit/patch.git/log/?ofs=450)

[ Parent | on HN ]
Re:A 35-year-old bug in Patch found in efforts to restore 29... by rurban on Monday August 17, 2020 @03:56PM (#24191271)
Nope. GNU Patch definitely is based on Larry's patch. But this bug is known since shar's need to workaround it. It's just that the GNU patch had that fixed very early on, BSD and probably others not. So shar still had to use the workaround.
[ Parent | on HN ]
Re:A 35-year-old bug in Patch found in efforts to restore 29... by bsdimp on Monday August 17, 2020 @04:58PM (#24191997)
Shar's workaround is for leading white space. This is change is for not assuming traling newlines.
[ Parent | on HN ]
Re:A 35-year-old bug in Patch found in efforts to restore 29... by bsdimp on Monday August 17, 2020 @04:57PM (#24191988)
It absolutely shared history with Larry Wall's patch.... It took the 2.0 sources and started improving them. I ran across a blurb about this somewhere. Also, the code structure is clearly derivative: variable names, function names, wording of messages.
[ Parent | on HN ]
Re:A 35-year-old bug in Patch found in efforts to restore 29... by asveikau on Monday August 17, 2020 @04:15PM (#24191499)
> I don’t think GNU patch shares history with Larry Wall’s original.

Seems extremely unlikely to me that someone would independently write that if statement almost identically, with the same comment. These are very clearly descended from the same source.

[ Parent | on HN ]
Re:A 35-year-old bug in Patch found in efforts to restore 29... by kazinator on Monday August 17, 2020 @04:17PM (#24191513)
... in a function having the same name, with a goto to an identical label name, and an error message string literal with exactly the same text, all in a file also named pch.c ...
[ Parent | on HN ]
Re:A 35-year-old bug in Patch found in efforts to restore 29... by LukeShu on Monday August 17, 2020 @10:15PM (#24194603)
Well, trivially:

    $ patch --version
    GNU patch 2.7.6
    Copyright (C) 2003, 2009-2012 Free Software Foundation, Inc.
    Copyright (C) 1988 Larry Wall

    License GPLv3+: GNU GPL version 3 or later <http://gnu.org/licenses/gpl.html>.
    This is free software: you are free to change and redistribute it.
    There is NO WARRANTY, to the extent permitted by law.

    Written by Larry Wall and Paul Eggert
But also, the GPL requires keeping a detailed ChangeLog, and if anyone's going to be a stickler about that, it's the FSF: https://git.savannah.gnu.org/cgit/patch.git/tree/ChangeLog-2...
[ Parent | on HN ]
Re:A 35-year-old bug in Patch found in efforts to restore 29... by bsdimp on Monday August 17, 2020 @03:12PM (#24190702)
True. I'd been looking at an older version of gnu patch when I thought it hadn't been patched to fix this... It looks like it had a while ago...
[ Parent | on HN ]