Re: bug #41707: RCS 5.8 file corruption when using file descriptor IO for large files

[Thien-Thi Nguyen]

() Achim Gratz <address@hidden>
() Sun, 10 Aug 2014 20:00:54 +0200

   [changes to src/z.c]
   I've made the data file some odd number of kiB so
   the various buffer sizes aren't obscured.

Just curious: What is its precise size now (per ‘ls -l’)?

   In doing these experiments I had an idea for a far less
   intrusive patch.  The code that rewinds the file before diff
   actually doesn't need to keep the stream and fd position the
   same before the runv call since runv only inherits the fd.
   Calling fro_bob _only after_ the runv call actually gets the
   stream into a useable state, while calling it before and after
   does apparently nothing (presumably because it already is at
   the same position).

Thanks for mentioning this.  It tickled my memory and i was able
to search src/ChangeLog for "sync" to ultimately trace the change
in behavior to:

 http://git.savannah.gnu.org/cgit/rcs.git/commit?id=cf7bab62a85

which went into RCS 5.7.94 (released 2010-11-08).  I think our
experiments w/ src/z.c yield a more conclusive test than the
configure-time test deleted in that change, so a simple revert is
not indicated.  That is, the simplification is partially correct.
What i ignorantly missed (originally, in 2010) was:

- De-sync is natural and expected
  (info "(libc) Stream/Descriptor Precautions")
  so removing all re-sync code represents a regression.

- The configure-time test for ‘CAN_FFLUSH_IN’ performs
  not only ‘fflush’ but also ‘lseek’ afterwards.

Anyway, does this patch (against branch ‘p’) work for you?

 src/ci.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/src/ci.c b/src/ci.c
index 97f949d..6f7e291 100644
--- a/src/ci.c
+++ b/src/ci.c
@@ -1092,6 +1092,11 @@ ci_main (const char *cmd, int argc, char **argv)
                 *++diffp = NULL;
                 if (DIFF_TROUBLE == runv (wfd, diffname, diffv))
                   RFATAL ("diff failed");
+
+                if (STDIO_P (work.fro)
+                    && PROB (lseek (wfd, 0, SEEK_SET)))
+                  Ierror ();
+
                 if (newhead)
                   {
                     fro_bob (work.fro);

This keeps the pre-diff(1) stream rewind and fixes up the file
descriptor post-diff(1) only.  It does not use ‘fflush’ as that is
presumably part of the stream rewind.  (Am i missing something?)

-- 
Thien-Thi Nguyen
   GPG key: 4C807502
   (if you're human and you know it)
      read my lisp: (responsep (questions 'technical)
                               (not (via 'mailing-list)))
                     => nil

signature.asc
Description: PGP signature