Ping[3]: [PATCH][BZ #11741] printf should return negative value on I/O error
Carlos O'Donell
carlos@systemhalted.org
Fri Nov 16 04:58:00 GMT 2012
On Thu, Nov 15, 2012 at 11:27 PM, Siddhesh Poyarekar
<siddhesh@redhat.com> wrote:
> On Thu, 15 Nov 2012 23:00:48 -0500, Carlos wrote:
>> Your original patch still suffers the same problem, you make
>> a change to libio/iowpadn.c that isn't needed?
>>
>> diff --git a/libio/iowpadn.c b/libio/iowpadn.c
>> index 05632d5..334e01a 100644
>> --- a/libio/iowpadn.c
>> +++ b/libio/iowpadn.c
>> @@ -65,7 +65,7 @@ _IO_wpadn (fp, pad, count)
>> w = _IO_sputn (fp, (char *) padptr, PADSIZE);
>> written += w;
>> if (w != PADSIZE)
>> - return written;
>> + return w <= 0 ? w : written;
>> }
>>
>> if (i > 0)
>> ---
>>
>> Can you confirm you need this change?
>>
>
> Ah, sorry, I have finally paged in the context of this patch. The
> changes are needed.
>
> _IO_file_xsputn can return EOF if overflow handling fails, i.e.
> flushing to the file fails:
So the patch is needed to propagate the EOF?
Isn't that a violation of what xsputn should do?
I assume we're following ISO C++'s definition of xsputn, in which
case it should return the number of bytes written.
> @@ -1330,9 +1331,10 @@ _IO_new_file_xsputn (f, data, n)
> _IO_size_t block_size, do_write;
> /* Next flush the (full) buffer. */
> if (_IO_OVERFLOW (f, EOF) == EOF)
> - /* If nothing else has to be written we must not signal the
> - caller that everything has been written. */
> - return to_do == 0 ? EOF : n - to_do;
> + /* If nothing else has to be written or nothing has been written, we
> + must not signal the caller that the call was even partially
> + successful. */
> + return (to_do == 0 || to_do == n) ? EOF : n - to_do;
This code, *and* the original code seems wrong, we shouldn't return EOF,
only the number of bytes written e.g. n - to_do.
Does the caller really need to know about the EOF?
>
> /* Try to maintain alignment: write a whole number of blocks.
> dont_write is what gets left over. */
>
> Of course, it doesn't currently since _IO_new_file_write (which is
> called in the overflow handling) fails to propagate the error, which
> this patch now fixes. In fact, I wasn't sure if xsputn returning 0 was
> expected since I was expecting it to return either EOF (0 or error) or
> the number of bytes read. Andreas' response reinforced that and hence
> the modified patch.
I expect xsputn to return the number of bytes written, including 0,
and never EOF, since that is what ISO C++ says.
If we aren't going to follow exactly what xsputn does in C++ then we
should add a very large warning about this.
Cheers,
Carlos.
More information about the Libc-alpha
mailing list