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

Jeff King [off-list ref] writes:

quoted

If we are trying to make sure that the caller would not say "failed
to close tempfile: ERRNO" with an ERRNO that is unrelated to any
stdio opration, I am not sure if the patch improves things.  The
caller did not fail to close (most likely we successfully closed
it), and no matter what futzing we do to errno, the message supplied
by such a caller will not be improved.

Right. EIO is almost certainly _not_ the error we saw. But I would
rather consistently say "I/O error" and have the user scratch their
head, look up this thread, and say "ah, it was probably a deferred
error", as opposed to the alternative: the user sees something
nonsensical like ENOMEM or EBADF. Those are more misleading, and worse,
may change from run to run based on what other code runs or fails in
between.

My point was actually not what errno we feed to strerror().  In that
example, what is more misleading is the fixed part of the error
message the caller of close_tempfile() used after seeing the funcion
fail, i.e. "failed to close".  strerror() part is used to explain
why we "failed to close", and of course any nonsensical errno that
we did not get from the failed stdio call would not explain it, but
a more misleading part is that we did not even "failed to close" it.

We just noticed an earlier error while attempting to close it.
strerror() in the message does not even have to be related to the
closing of the file handle.

quoted

If the caller used "noticed an earlier error while closing tempfile:
ERRNO", such a message would describe the situation more correctly,
but then ERRNO that is not about stdio is probably acceptable in the
context of that message (the original ERRNO might be ENOSPC that is
even more specific than EIO, FWIW).  So I am not sure if the things
will improve from the status quo.

Yes, that's I suggested that xfclose() is probably not a good direction.
The _best_ thing we can do is have the caller not report errno at all
(or even say "there was an earlier error, I have no idea what errno
was"). And xfclose() works in the opposite direction.

I think we are in agreement on this point ;-)

The only reason I do not think we should do so for close_tempfile() is
that the fclose is typically far away from the code that actually calls
error(). We'd have to pass the tristate (success, fail, fail-with-errno)
state up through the stack (most of the calls indirectly come from
commit_lock_file(), I would think).

We _could_ clear errno to allow caller to tell them apart, though,
if we wanted to ;-)