[PATCH] prevent retries on fclose/fflush after write errors

[Denys Vlasenko]

On Friday 11 February 2011 19:18, Bernhard Reutner-Fischer wrote:
> "Denys Vlasenko" <vda.linux at googlemail.com> wrote:
> 
> >Currently, uclibc retains buffered data on stdio write errors,
> >and subsequent fclose and fflush will try to write it out again
> >(in most cases, in vain).
> >
> >Which results in something like this:
> >
> >On Wednesday 26 January 2011 13:21, Baruch Siach wrote:
> >> Hi busybox list,
> >> 
> >> I'm running the following command under strace (thanks Rob):
> >> 
> >> echo 56 > /sys/class/gpio/export
> >> 
> >> and I see the following output:
> >> 
> >> write(1, "56\n", 3)                     = -1 EBUSY (Device or
> >resource busy)
> >> write(1, "5", 1)                        = 1
> >> 
> >> The first EBUSY is OK, since GPIO 56 is already requested. But the
> >second 
> >> write() attempt seems strange, and leads to an unwanted outcome. GPIO
> >5 gets 
> >> exported.
> >
> >
> >This patch prevents that.
> >
> >-- 
> >vda
> >
> >diff -d -urpN uClibc.0/libc/stdio/_wcommit.c
> >uClibc.1/libc/stdio/_wcommit.c
> >--- uClibc.0/libc/stdio/_wcommit.c	2011-02-07 00:04:34.000000000 +0100
> >+++ uClibc.1/libc/stdio/_wcommit.c	2011-02-07 00:55:24.000000000 +0100
> >@@ -20,7 +20,18 @@ size_t attribute_hidden __stdio_wcommit(
> > 
> > 	__STDIO_STREAM_VALIDATE(stream);
> > 
> >-	if ((bufsize = __STDIO_STREAM_BUFFER_WUSED(stream)) != 0) {
> >+	/* Note: we do not write anything if write error has been detected.
> >+	 * Otherwise, stdio user has no way to prevent retries after
> >+	 * failed write - and some users do want to not have any retries!
> >+	 * IOW: if write errored out, neither fflush nor fclose should
> >+	 * try to write buffered data.
> >+	 * clearerr may be used to enable retry if needed.
> >+	 */
> >+
> >+	bufsize = __STDIO_STREAM_BUFFER_WUSED(stream);
> >+	if (bufsize != 0
> >+	 && !(stream->__modeflags & __FLAG_ERROR)
> >+	) {
> > 		stream->__bufpos = stream->__bufstart;
> > 		__stdio_WRITE(stream, stream->__bufstart, bufsize);
> > 	}
> 
> Hi,
> 
> Sounds plausible (but I did not check the standard), please install.

It turned out that fixing just fflush/fclose isn't enough:
it doesn't fix the problem when we *continue* to write
to this FILE* (as it happens in busybox shells).

I ended up dropping FILE-based output in echo,
and used fd-based io.

IOW: for the particular bug I was working on,
this fix is no longer needed.

Do you think it still needs to be applied?

-- 
vda