Re: [PATCH] tempfile: avoid "ferror | fclose" trick

From: Jeff King <peff@peff.net>
Date: Feb 17, 2017, 21:21 UTC
Message-ID: <20170217212106.bew6krtb7pqpi3rr@sigill.intra.peff.net>
In-Reply-To: <xmqq37fcsejx.fsf@gitster.mtv.corp.google.com>

On Fri, Feb 17, 2017 at 01:17:06PM -0800, Junio C Hamano wrote:

Show 11 quoted lines

> > I guess we are simultaneously assuming that it is OK to munge errno on
> > success in our function, but that fclose() will not do so. Which seems a
> > bit hypocritical. Maybe the "if" dance is better.
> 
> Yes.  When both ferror() and fclose() are successful, we would
> prefer to see the original errno unmolested, so the "if" dance,
> even though it looks uglier, is better.  The ugliness is limited
> to the implementation anyway ;-)
> 
> But it does look ugly, especially when nested inside the existing
> code like so.

I think our emails crossed, but our patches are obviously quite similar. My commit message maybe explains a bit more of my thinking.

> Stepping back a bit, would this be really needed?  Even if the ferror()
> does not update errno, the original stdio operation that failed
> would have, no?

Sure, but we have no clue what happened in between.

-Peff