Jeff King <peff@xxxxxxxx> writes: >> 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. >> 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 ;-)