Re: [PATCH] head,tail: consistently diagnose write errors

[Pádraig Brady]

On 01/29/2014 11:12 PM, Bernhard Voelker wrote:
> On 01/30/2014 12:02 AM, Bernhard Voelker wrote:
>> On 01/29/2014 07:43 PM, Pádraig Brady wrote:
>>> On 01/29/2014 12:06 PM, Pádraig Brady wrote:
>>>
>>>> --- a/NEWS
>>>> +++ b/NEWS
>>>
>>>> +  tail now diagnoses all failures when writing to stdout.  Previously 
>>>> write
>>>> +  errors could have been silently ignored if some data was output.
>>>> +  [This bug was present in "the beginning".]
>>>
>>> Actually this is not the case. The atexit handler would have caught
>>> partial write failures, albeit with a less detailed error message.
>>> So I'll remove the NEWS but leave the code changes.
>>
>> Thanks.
>>
>> It seems your patch didn't make it thru the mail systems:
>>
>>   error: patch failed: src/head.c:504
>>   error: src/head.c: patch does not apply
>>   Patch failed at 0001 head,tail: consistently diagnose write errors
>>
>> Can you resend as attachment, please?
> 
> Sorry, I missed that this patch was based on the other patch for
> bug http://bugs.gnu.org/16329. After that, it applies cleanly.
> 
>> However, if head is exiting earlier on the first write error,
>> then there should be a user visible change.
>>
>>   $ seq -f'%1000g' 1000 | { src/head --lines=-1 > /dev/full ; wc -l ; }
>>   src/head: write error
>>   0
>>
>> With the patch, I'd expect wc to see more than 0 lines in the above
>> case, right?
> 
> Indeed, now with the patch:
> 
>   $ seq -f'%1000g' 1000 | { src/head --lines=-1 > /dev/full ; wc -l ; }
>   src/head: write error: No space left on device
>   src/head: write error
>   984

Good point.
I don't think this could cause issues though, as if you're
reliant on position adjustments by head(1), then you should
be doing `head ... && ....` rather than `head ... ; ...`
Or looking at it another way, if head(1) can't do the
output operations asked of it, is there any point proceeding
with the input operations?
So we probably don't need to document this change in behavior.

> Unfortunately, the atexit code now produces a second error diagnostic.
> It doesn't hurt, but it looks a bit ugly.

We discussed that foible previously, and thought it not worth
adding the extra complexity to avoid in general.

http://lists.gnu.org/archive/html/coreutils/2011-05/msg00061.html

Though in this specific case we're using wrapper functions
instead of fwrite() so we can easily add the clearerr() there.

Updated patch attached.

thanks!
Pádraig.

head-tail-write-error.patch
Description: Text Data