#268 patch silently skips trailing diffs if long, piped to stdin

[Joel VanderZee]

I am using the GnuWin32 patch 2.5.9 under Windows XP.
There is a bug that causes silent misoperation for some
common input files when piped to input.

** Symptom:

Patch correctly applies hunks from at least one diff in
the file, but exits normally (says "done" with
--verbose) without applying one or more remaining diffs
in the file.

** How to Reproduce:

1) Create a patch file such that NL > NC where:
NL is the number of lines in the file up to the last
line of a diff that will be applied and
NC is the number of characters/bytes in the rest of the
file including diffs that will be missed

2) Attempt to apply the patch by piping to stdin:
cat <test_patch_file> | patch -d <directory> -p1 --verbose

Note that this is a common type of patch file,
resulting whenever the last file alphabetically has a
small change, following diffs of sufficient number and
length. Or it could be a long email message with more
than one file diff at the end. One reason someone would
pipe input to patch would be to apply several
successive patches in one operation by concatenating
the patch files in the right order as one input -- so
the size of the patch input is the combined size of the
successive patches.

** Confirmed Workaround:

Use input redirection or specify input file as argument.
patch -d <directory> -p1 < <test_patch_file>

** Suspected Cause:

See source code file pch.c.

At line 206, if pointer into file exceeds file length,
say "done" instead of looking for more diffs. OK.

At line 161, filesize is taken from stat results
structure. OK.

At lines 141 to 143, for stdin not a regular file, the
size in the stat results structure is overwritten by
counting charsread from the fread function while
creating the temporary file. I confirmed that the
temporary file is created correctly, an exact copy of
the input, but I suspect that fread reports the wrong
charsread.

I found one version of fread doc that is confusing. It
says that CR/LF is replaced by a single newline
character for a file opened in text mode, but it claims
that the count returned and the file position are not
affected by the substitution. Similarly, fwrite
substitutes CR/LF for a newline on the way out. It
seems to me that the count is in fact affected to
represent the actual number of bytes written to the
buffer, but a byte is missing in the buffer for every
end-of-line pair of bytes in the files. Everything
would work fine except that the file position pointer
counts two bytes for end-of-line.

Specifying the --binary flag does not work around the
problem, because the files are in fact text. The patch
fails to apply on every hunk with --binary specified.

** Proposed Fix:

My first proposal was to apply the patch below.
However, in email with patch maintainer Paul Eggert,
bug-patch@gnu.org, I proposed another fix that he
thought was better.

I do not have tools to build and test either fix, and
Paul needs a tested change to implement.

The fix Paul prefers is to remove the character
counting and add a "file_tell(pfp)" before seeking back
to the beginning of the temporary file in line 149 (pch.c).

Here's the other possible fix, to fstat the temporary file:

--- pch.c 2005-03-23 21:28:44.000000000 -0600
+++ pch_proposed.c 2005-06-23 09:40:15.343511200 -0500
@@ -138,9 +138,7 @@
printf ("TMPPATNAME: %s\n", TMPPATNAME);
if (!pfp)
pfatal ("Can't open stream for file %s",
quotearg (TMPPATNAME));
- for (st.st_size = 0;
- (charsread = fread (buf, 1, bufsize, stdin)) != 0;
- st.st_size += charsread)
+ for ( ; (charsread = fread (buf, 1, bufsize,
stdin)) != 0; )
if (fwrite (buf, 1, charsread, pfp) != charsread)
write_fatal ();
if (ferror (stdin) || fclose (stdin) != 0)
@@ -148,6 +146,8 @@
if (fflush (pfp) != 0
|| file_seek (pfp, (file_offset) 0, SEEK_SET) != 0)
write_fatal ();
+ if (fstat (fileno (pfp), &st) != 0)
+ pfatal ("fstat");
}
}
else

[GnuWin]

Logged In: YES
user_id=217802

I'm not sure I understand well enough the description of the
type of patch file. Could you perhaps provide a small test file
that exhibits this bugs?

[Joel VanderZee]

a diff file against patch-2.5.9-src demonstrating bug

[Joel VanderZee]

Logged In: YES
user_id=1313457

I'll assume that you have the source for patch 2.5.9, so I
can provide a diff file to apply against it to demonstrate
the failure. All that this patch does is rearrange functions
in pch.h and modify the version function.

Here's what happens when I run it:
C:...>cat bug_diff.txt | patch -d patch-2.5.9-src-copy -p1
TMPPATNAME: C:...\Temp/ppwjvxkc
patching file pch.c

C:...

But if you look at the end of the file, you will see that it
includes a diff for version.c as well. Version.c is not
patched, and there is no error message. If you rearrange the
file so that the version.c diff comes first, then both files
will be patched correctly.

All you need is patch file with more lines in the part that
works than the characters in the rest of the file (that will
be missed). It's probably not hard to find examples. Any
project with a large enough change where the last file is
something like "version.h" with a one-line change will fail
to patch correctly (under Windows).

[GnuWin]

Logged In: YES
user_id=217802

This bug has been introduced by my trying to solve
the "Assertion failed, hunk, file patch.c, line 343"-bug.
Inadvertently, I left some changes in pch.c and inp.c, and
these cause the bug you have described. So, there is no
need to correct the original sources, and I'll upload a new
port. I'm sorry for the confusion this has created and for the
trouble you have taken to spot the cause. Anyway, thanks for
reporting this.

[GnuWin]

Logged In: YES
user_id=217802

This bug has been fixed. A new release is available through
http://gnuwin32.sourceforge.net/