git/list[1] front-page[2] threads[3] people[4] search[5] about
 

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

From
Junio C Hamano <gitster@pobox.com>
Date
Feb 17, 2017, 23:52 UTC
Message-ID
<xmqqr32wnznf.fsf@gitster.mtv.corp.google.com>
In-Reply-To
<20170217233952.56h2z3l76orn4zht@sigill.intra.peff.net>
Jeff King <peff@peff.net> writes:
Show 5 quoted lines
> 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.
Show 13 quoted lines
>> 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.
Show 12 quoted lines
> 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' ;-).

Previous: Jeff KingNext: Jeff King
Message 23 of 24 in “Confusing git messages when disk is full.”
  1. Jáchym BarvínekFeb 12, 2017
  2. Jeff KingFeb 15, 2017
  3. Junio C HamanoFeb 15, 2017
  4. Jeff KingFeb 15, 2017
  5. Junio C HamanoFeb 15, 2017
  6. Jeff KingFeb 15, 2017
  7. Junio C HamanoFeb 15, 2017
  8. Jeff KingFeb 15, 2017
  9. Andreas SchwabFeb 16, 2017
  10. Jeff KingFeb 16, 2017
  11. tempfile: avoid "ferror | fclose" trickJeff King, Feb 16, 2017
  12. Michael HaggertyFeb 17, 2017
  13. Jeff KingFeb 17, 2017
  14. Michael HaggertyFeb 17, 2017
  15. Jeff KingFeb 17, 2017
  16. Jeff KingFeb 17, 2017
  17. Junio C HamanoFeb 17, 2017
  18. Jeff KingFeb 17, 2017
  19. Junio C HamanoFeb 17, 2017
  20. Jeff KingFeb 17, 2017
  21. Junio C HamanoFeb 17, 2017
  22. Jeff KingFeb 17, 2017
  23. Junio C HamanoFeb 17, 2017
  24. Jeff KingFeb 17, 2017

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.