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

Bernhard Reutner-Fischer rep.dot.nop at gmail.com
Mon Feb 18 19:58:43 UTC 2013


On 25 March 2012 07:54, Mike Frysinger <vapier at gentoo.org> wrote:
> fixed the style & pushed.  thanks all!  it's nice we have people versed in
> esoteric low level aspects nowadays.

hmz, revisiting..
To recap:

On 7 February 2011 02:41, 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.

This rather sounds to me that this second write with an odd length is wrong, no?
IIUC the patch (867bac0c) that went in for this issue is causing
https://bugs.uclibc.org/5156 (fclose and fflush do not report write failures)

I am attaching denys.c, i think this is the testcase you had in mind
in your patch that Mike applied as
867bac0c750401d2f429ad6bb066498c3b8b35c1
The attached fulltest.c is from above mentioned PR5156.
Also attached is a patch that seems to cure PR5156 (the hunk against
_wcommit.c is your rephrased patch cited below, not needed for 5156
though) and that basically reverts your 867bac0c.
With this patch we would get:
$ ./denys_uc > /dev/null
Hi there 1
Hi there 2
$ ./denys_uc 2> /dev/null
$ ./denys_glibc > /dev/null
Hi there 1
Hi there 2
$ ./denys_glibc 2> /dev/null
$ ./fulltest_glibc
fwrite: x=4, errno=25
fflush: y=-1, errno=28
fputc 1: x=88, errno=25
fflush: y=-1, errno=28
fputc 2: x=88, errno=25
fclose: y=-1, errno=28
# the ENOTYPEWRITER i dislike
$ ./fulltest
fwrite: x=4, errno=0
fflush: y=-1, errno=28
fputc 1: x=88, errno=28
fflush: y=-1, errno=28
fputc 2: x=88, errno=28
fclose: y=-1, errno=28
so 5156 would be fixed, we'd diverge from glibc in the type-writer
errno (which is imo ok) and (since we fully buffer fwrite) do not
errno the fwrite.
I guess it would not fix your (line-buffered, i assume?) gpio echo, would it?

> --
> 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);
>         }


More information about the uClibc mailing list