Re: Undetected fatal errors from redirected print

[Miguel Pineiro Jr.]

Hi, Andy

On Fri, Nov 26, 2021, at 11:28 AM, Andrew J. Schorr wrote:
> Interesting. Do we actually need the goto in efwrite, or can we just
> skip the fwrite call if count is 0? That way, we can avoid the ugliness
> of testing the first character of `from'. See the attached patch.

Testing the first character of from, I agree, is ugly. Though effective, it's 
cryptic and arbitrary.

The goto isn't necessary, but I think it actually makes the code and the intent 
very easy to follow: If this condition is true, jump to the flush.

I see two issues with your patch:

1. If I understand your intent, efwrite should flush when count is 0, but it 
doesn't. The flushing conditional expression needs OR-ing with "count == 0". 

2. do_printrec, do_print, and do_printf may call efwrite with a zero count. In 
those instances,
flushing would be forced where before it never occurred (fwrite with a zero 
count is a no-op).

If a parameter were to be overloaded, perhaps 'size' would be a better choice? 
Every extant call to efwrite uses a size of sizeof(char). Never a size of 0. 
Or, to be even stricter, you could require count and size to both be zero.

> The close_io logic needs a closer look, since there's already some
> specialized error-handling code there. Is it clear that it's obsoleted
> by calling efwrite instead?

I think it is. efwrite's error handling checks everything close_io is checking 
and more. efwrite checks ferror after fflush; close_io does not. efwrite 
respects a descriptor's nonfatal setting; close_io does not. Both differences 
are close_io deficiencies.

Take care,
Miguel