Re: Undetected fatal errors from redirected print

[Andrew J. Schorr]

On Fri, Nov 26, 2021 at 03:51:07AM -0500, Miguel Pineiro Jr. wrote:
> It occurred to me, why not allow efwrite to finish what it started. At
> stream close, it can be called to flush its data and handle its errors.
> 
> Presently, print statement data may be written by efwrite, close_rp,
> and close_io (stdout), each with a different notion of what should
> be done when things don't go according to plan. It would be ideal
> if all print statement output were handled by a single code path,
> yielding consistent error handling and messaging.
> 
> Below you'll find a crude, minimal, proof-of-concept diff. The
> changes compiled cleanly, `make check` passed all tests, and my
> initial report's test script produced the expected output.
> 
> I overload efwrite's 'from' parameter. If the name begins with an
> asterisk, it signals that the caller is closing the stream; there's
> no new data to print; and that efwrite must unconditionally flush
> the stream and handle any errors.
> 
> Though I appreciate its succinctness, I realize it's a hack. If
> the idea of using efwrite sounds good, but this approach does not,
> there are other options. Perhaps efwrite could gain a parameter. Or,
> efwrite's flushing and error handling code could be extracted into
> a function for use in efwrite, close_redir, and close_io.
> 
> To improve this patch, I'd delete the stdout handling code in
> close_io which the call to efwrite makes redundant. I'd also add
> commentary. I did neither to keep the patch as small as possible,
> in hopes of staying below the copyright assignment threshold. If
> someone wants to run with this, I'm sure they can whip it into shape.
> 
> This patch is based on the last version before the autotools bump in
> September: a08cc63b

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.

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? This aspect looks a bit messy.

Regards,
Andy

efwrite.patch
Description: Text document