Re: bug #41707: RCS 5.8 file corruption when using file descriptor IO for large files
Achim Gratz <[email protected]> Sun, 10 Aug 2014 20:00:54 +0200
| Newsgroups | gmane.comp.version-control.rcs.bugs |
|---|---|
| Organization | Linux Private Site |
| Message-ID | <[email protected]> |
Thien-Thi Nguyen writes: > I have added ephemeral branch "q-spurious-eof" w/ file src/z.c > (see comments therein). I'm not confident that it implements what > you described; you will certainly have to debug it further. Once > we get it to reliably demonstrate buggy behavior (exit value 5), > i'll convert it to a configure-time test. I've had to change a few things to make it run (you can't execve with a NULL environment on Windows since the loader won't find the libraries then). Also I've tried to make it conform more closely to what ci does through calling runv. In the end I could finally demonstrate the failure (exit value 4, actually). I've made the data file some odd number of kiB so the various buffer sizes aren't obscured. The child must read at least 64kiB for the error to occur (that's what a stream buffers, I think). If the child reads to EOF (the commented loop variant), then the parent just ends the first read with zero bytes, i.e. it doesn't see the EOF until it actually tries to read. 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). Regards, Achim. -- +<[Q+ Matrix-12 WAVE#46+305 Neuron microQkb Andromeda XTk Blofeld]>+ SD adaptation for Waldorf microQ V2.22R2: http://Synth.Stromeko.net/Downloads.html#WaldorfSDada
0001-fix-demonstrator-and-provide-a-different-patch-for-c.patch
(text/x-patch, 5.7 KB)
From 4358795cf083449ab499b5b05f50198bd116b8b5 Mon Sep 17 00:00:00 2001 From: Achim Gratz <[email protected]> Date: Sun, 10 Aug 2014 19:15:06 +0200 Subject: [PATCH] fix demonstrator and provide a different patch for ci --- src/ci.c | 26 +++------------------ src/z.c | 81 +++++++++++++++++++++++++++++++++++++++++++++++++++------------- 2 files changed, 68 insertions(+), 39 deletions(-) diff --git a/src/ci.c b/src/ci.c index 72ddccd..3207133 100644 --- a/src/ci.c +++ b/src/ci.c @@ -1069,8 +1069,7 @@ ci_main (const char *cmd, int argc, char **argv) SAME_AFTER (from, bud.target->text); bud.d.pretty_log = getlogmsg (&reason, &bud); - /* "Rewind" ‘work.fro’ before feeding it to diff(1). */ - fro_bob (work.fro); + /* "Rewind" wfd before feeding it to diff(1). */ if (PROB (lseek (wfd, 0, SEEK_SET))) Ierror (); @@ -1086,27 +1085,8 @@ ci_main (const char *cmd, int argc, char **argv) if (DIFF_TROUBLE == runv (wfd, diffname, diffv)) RFATAL ("diff failed"); -#if 0 /* "tiny change" contributed by Achim Gratz: - <http://lists.gnu.org/archive/html/bug-rcs/2014-06/msg00000.html> - - status: on track to be installed, pending: - - configure-time check - - selection of appropriate #define name (to replace ‘#if 0’) - - ChangeLog entry - */ - - /* Re-open the work file to sync the notion of the - file position and the memory buffer. These may - have drifted apart due to sharing the fd with the - diff subprocess. */ - fro_zclose (&work.fro); - if (!(work.fro = fro_open (mani_filename, FOPEN_R_WORK, &work.st))) - { - syserror_errno (mani_filename); - continue; - } - -#endif + /* "Rewind" ‘work.fro’ after feeding it to diff(1). */ + fro_bob (work.fro); if (newhead) { diff --git a/src/z.c b/src/z.c index bef5bc2..41134f7 100644 --- a/src/z.c +++ b/src/z.c @@ -28,7 +28,7 @@ 1 -- unused 2 -- syscall failed 3 -- child problem - 4 -- unused + 4 -- parent doesn't read the whole file 5 -- parent sees EOF from child Results (platform, exit value, output): @@ -75,8 +75,37 @@ |cgot[29]: 0 => 114688 |status: 0 - * "cygwin" -- ??? (we would expect 5) - | ??? + * "cygwin" -- 4 + |argc: 1 + |argv[0]: ./z + |fd: 3 + |pgot[1]: 8192 => 8192 + |pgot[2]: 8192 => 16384 + |pgot[3]: 8192 => 24576 + |pgot[4]: 8192 => 32768 + |pgot[5]: 8192 => 40960 + |argc: 2 + |argv[0]: ./z + |argv[1]: child + |cgot[1]: 8192 => 8192 + |cgot[2]: 8192 => 16384 + |cgot[3]: 8192 => 24576 + |cgot[4]: 8192 => 32768 + |cgot[5]: 8192 => 40960 + |cgot[6]: 8192 => 49152 + |cgot[7]: 8192 => 57344 + |cgot[8]: 8192 => 65536 + |cgot[9]: 8192 => 73728 + |status: 0 + |pgot[1]: 8192 => 8192 + |pgot[2]: 8192 => 16384 + |pgot[3]: 8192 => 24576 + |pgot[4]: 8192 => 32768 + |pgot[5]: 8192 => 40960 + |pgot[6]: 8192 => 49152 + |pgot[7]: 1024 => 50176 + |truncated read! + */ #include <stdlib.h> @@ -99,7 +128,7 @@ child has produced. */ -#define BUF_SIZE 4096 /* fixme */ +#define BUF_SIZE 8192 /* fixme */ int fd; @@ -118,7 +147,7 @@ parent (void) pid_t pid; FILE *st; char buf[BUF_SIZE]; - size_t got; + off_t fend, sofar = 0; fd = open ("z.data", O_RDWR); if (0 > fd) @@ -128,16 +157,22 @@ parent (void) st = fdopen (fd, "r+"); if (! st) pbad ("fdopen"); + fseek (st, 0, SEEK_END); + fend = ftello (st); + fseek (st, 0, SEEK_SET); - for (int i = 1; i < 5; i++) + for (int i = 1; i < 6; i++) { - got = fread (buf, 1, sizeof (buf), st); - fprintf (stderr, "pgot[%d]: %zu\n", i, got); + off_t got = fread (buf, 1, sizeof (buf), st); + sofar += got; + fprintf (stderr, "pgot[%d]: %zu => %zu\n", i, got, sofar); } + fseeko (st, 0, SEEK_SET); + sofar = 0; - pid = fork (); + pid = vfork (); if (0 > pid) - pbad ("fork"); + pbad ("vfork"); if (! pid) /* child */ @@ -148,10 +183,10 @@ parent (void) if (0 > res) pbad ("dup2"); - res = execve (cmd[0], cmd, NULL); + res = execv (cmd[0], cmd); /* should not get here */ fprintf (stderr, "res: %d\nerrno: %d\n", res, errno); - pbad ("execve"); + pbad ("execv"); } else /* parent */ @@ -162,12 +197,26 @@ parent (void) if (pid != same) pbad ("stray child!"); + //fseeko (st, 0, SEEK_SET); + fprintf (stderr, "status: %d\n", status); if (feof (st)) { fprintf (stderr, "st at EOF!\n"); exit (5); } + for (int i = 1; ! feof (st); i++) + { + off_t got = fread (buf, 1, sizeof (buf), st); + + sofar += got; + fprintf (stderr, "pgot[%d]: %zu => %zu\n", i, got, sofar); + } + } + if (fend != sofar) + { + fprintf (stderr, "truncated read!\n"); + exit (4); } return EXIT_SUCCESS; @@ -184,7 +233,7 @@ int child (void) { char buf[BUF_SIZE]; - size_t sofar = 0; + off_t sofar = 0; FILE *st; if (0 != fd) @@ -193,14 +242,14 @@ child (void) cbad ("unepected fd"); } st = fdopen (fd, "r"); - for (int i = 1; ! feof (st); i++) + //for (int i = 1; ! feof (st); i++) + for (int i = 1; i < 10; i++) { - size_t got = fread (buf, 1, sizeof (buf), st); + off_t got = fread (buf, 1, sizeof (buf), st); sofar += got; fprintf (stderr, "cgot[%d]: %zu => %zu\n", i, got, sofar); } - return EXIT_SUCCESS; } -- 2.0.4