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

[Bernhard Reutner-Fischer]

attaching..

On 18 February 2013 20:58, Bernhard Reutner-Fischer
<rep.dot.nop at gmail.com> wrote:
> 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);
>>         }
-------------- next part --------------
A non-text attachment was scrubbed...
Name: denys.c
Type: text/x-csrc
Size: 390 bytes
Desc: not available
URL: <http://lists.busybox.net/pipermail/uclibc/attachments/20130218/7adc2329/attachment.c>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: fulltest.c
Type: text/x-csrc
Size: 980 bytes
Desc: not available
URL: <http://lists.busybox.net/pipermail/uclibc/attachments/20130218/7adc2329/attachment-0001.c>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: uClibc-redo-867bac0c750401d2f429ad6bb066498c3b8b35c1.00.patch
Type: application/octet-stream
Size: 2001 bytes
Desc: not available
URL: <http://lists.busybox.net/pipermail/uclibc/attachments/20130218/7adc2329/attachment.obj>