Jeff King <peff@xxxxxxxx> writes: > On Fri, Feb 17, 2017 at 01:17:06PM -0800, Junio C Hamano wrote: > >> 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. Hmm, so we are protecting against somebody who does "errno = 0" explicitly, because she knows that she's dealt with the error from stdio earlier? Such a careful person would have called clearerr() as well, I would guess. So let's assume we only care about the case where some other error was detected and errno was updated by a system library call. > I think our emails crossed, but our patches are obviously quite similar. > My commit message maybe explains a bit more of my thinking. Yes, but ;-) 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. 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. It's easy for us to either apply the patch and be done with it (or drop and do the same), and in the bigger picture it wouldn't make that much of a difference, I would think, so I can go either way.