Re: [PATCH 2/2] tee: add --write-error to control handling of closed pipes

[Pádraig Brady]

On 26/02/15 07:55, Bernhard Voelker wrote:
> On 02/17/2015 03:44 AM, Pádraig Brady wrote:
>> To improve the supported combinations we add these options:
>>
>>  --write-error=warn
>>    Warn if error writing any output including pipes.
>>    Allows continued writing to still open files/pipes.
>>    Exit status is failure if any output had error.
>>  --write-error=warn-nopipe, -p
>>    Warn if error writing any output except pipes.
>>    Allows continued writing to still open files/pipes.
>>    Exit status is failure if any non pipe output had error.
>>  --write-error=exit
>>    Exit if error writing any output including pipes.
>>  --write-error=exit-nopipe
>>    Exit if error writing any output except pipes.
> 
> I know I'm a bit late on this.  I like it and the patch looks good
> to me.  Just 2 comments.
> 
>> diff --git a/NEWS b/NEWS
>> index b6795aa..0ccadde 100644
>> --- a/NEWS
>> +++ b/NEWS
>> @@ -58,6 +58,8 @@ GNU coreutils NEWS                                    -*- 
>> outline -*-
>>    sync no longer ignores arguments, and syncs each specified file, or with 
>> the
>>    --file-system option, the file systems associated with each specified 
>> file.
>>  
>> +  tee accepts a new --write-error option to control operation with pipes.
>> +
> 
> As this new option can also changes the exit behavior, i.e., exit early,
> in the case of no pipes ...
> 
>   $ seq 10 | src/tee --write-error=exit /dev/full > /dev/null
>   src/tee: /dev/full: No space left on device
> 
> ... shouldn't the NEWS enrty mention this, too?

Yep, s/pipes/pipes and general output errors/

> 
> I stumbled upon another gap in the POSIX specification: it does not
> mention how tee(1) should react when another error than a write() error
> happens.  Well, it should exit with a an error value, that's clear.
> But as it already special-cases that tee should continue writing to
> other outputs when a write error has happened, I think POSIX should
> also clarify other errors. E.g. open(1) errors:
> 
>   mkdir dir
>   seq 10 | src/tee dir >/dev/null
>   src/tee: dir: Is a directory
> 
> Currently, tee also continues after open() errors - which I probably
> wouldn't expect as a user.

Very good point.
Perhaps we should generalize this to --on-error to cater for all cases?

I'm away for a few days now.

later,

thanks for the review,
Pádraig.