Thread (2 messages) flat view 2 messages, 2 authors, 2017-02-18

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' ;-).
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help