[PATCH] prevent retries on fclose/fflush after write errors
dalias at aerifal.cx
Tue Mar 13 06:06:10 UTC 2012
On Tue, Mar 13, 2012 at 01:43:36AM -0400, Mike Frysinger wrote:
> On Sunday 11 March 2012 20:50:27 Denys Vlasenko wrote:
> > On Sunday 11 March 2012 18:10, Rich Felker wrote:
> > > On Sun, Mar 11, 2012 at 04:12:19PM +0100, Denys Vlasenko wrote:
> > > > On Sunday 11 March 2012 14:46, Denys Vlasenko wrote:
> > > > > I propose the following patch.
> > > > > Only compile tested by me, care to run-test?
> > > >
> > > > A more correct (I hope) version:
> > > I'm curious why this is such a big patch introducing new functions and
> > > struct members. Couldn't it be just a one-line change in the function
> > > that writes out the buffers (i.e. do the buffer-voiding as soon as the
> > > error is hit rather than testing for it later)?
> > EINTR and EAGAIN should not result in output buffers being dropped.
> > You need to remember the error code for this.
> should not, but fairly certain POSIX allows us to. i'd note that glibc
> handles EINTR/EAGAIN in their fwrite/fread loops so that higher code doesn't
> have to write "safe_fwrite" like they do with "safe_read". would be nice if
> uClibc could provide this too if it's not too much overhead.
Looping in fread/fwrite when EINTR/EAGAIN is encountered is definitely
wrong and harmful behavior. If the application has installed an
interrupting signal handler, the intent is to allow the signal to
break out of blocked IO operations. And likewise if the file
descriptor is in nonblocking mode, the idea is that the application
would rather get an error than block. Since POSIX specifies these
errors for fgetc/fputc (and thereby for all stdio functions defined in
terms of these two), I think it's clearly wrong for an implementation
to hide them behind retry loops.
Now, if the implementation wants to avoid throwing away buffered data
in these cases, that's another matter; at least it doesn't break apps.
Note that I'm claiming the whole "safe_read"/"safe_write" idiom is
wrong, and a throwback to the pre-sigaction SysV era when signal() was
the only way to install a signal handler and it gave you
non-restarting semantics. The idea of an interrupting signal, when set
intentionally by a modern application using sigaction, is that EINTR
should be treated as a (usually permanent/unrecoverable) failure on
blocked in-progress IO operations.
More information about the uClibc