Re: [PATCH] tempfile: avoid "ferror | fclose" trick
From: Junio C Hamano <hidden>
Date: 2017-02-18 00:00:35
Jeff King [off-list ref] writes:
I'm also not sure that it's all that useful to distinguish errors from fwrite() versus fclose(). In practice, errors will come from write() in either case, and the caller does not have much control over when the flushing happens. So any error saying "error closing file" is probably assuming too much anyway. It should be "error writing file".
Yes.
quoted
We _could_ clear errno to allow caller to tell them apart, though, if we wanted to ;-)Hmm. So basically "errno = 0" instead of EIO? That at least has the advantage that you can tell it apart from a real EIO, and a caller _could_ if they chose do: if (commit_lock_file(&lock)) { if (!errno) error("error writing to file"); else error_errno("error closing file"); }
Exactly.
But I am not sure I would want to annotate all such callers that way. It would probably be less bad to just pass down a "quiet" flag or a strbuf and have the low-level code fill it in. And that solves this problem _and_ the rename() thing above. But TBH, I am not sure if it is worth it. Nobody is complaining. This is just a thing we noticed. I think setting errno to EIO or to "0" is a strict improvement over what is there (where the errno is essentially random) and the complexity doesn't escape the function. So I think that "easy" thing falls far short of a solution, but it's at least easy. I could take it or leave it at this point.
Well, I already said that earlier in this thread, and ended up queuing your patch on 'pu' ;-).