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

Re: [PATCH 3/7] read_in_full: reset errno before reading

From
Jeff King <peff@peff.net>
Date
Sep 25, 2017, 23:45 UTC
Message-ID
<20170925234541.l6qcislhfwtnxf5m@sigill.intra.peff.net>
In-Reply-To
<20170925233732.4lqavl56qwpfjuxy@sigill.intra.peff.net>
On Mon, Sep 25, 2017 at 07:37:32PM -0400, Jeff King wrote:
Show 12 quoted lines
> > Correct.  Actually more than "frown on": except for with the few calls
> > like strtoul that are advertised to work this way, POSIX does not make
> > the guarantee the above code would rely on, at all.
> > 
> > So it's not just frowned upon: it's so unportable that the standard
> > calls it out as something that won't work.
> 
> Is it unportable? Certainly read() is free reset errno to zero on
> success. But is it allowed to set it to another random value?
> 
> I think we're getting pretty academic here, but I'm curious if you have
> a good reference.
Answering my own question. POSIX says:
  No function in this volume of IEEE Std 1003.1-2001 shall set errno to
  0. The setting of errno after a successful call to a function is
  unspecified unless the description of that function specifies that
  errno shall not be modified.
So that does seem to outlaw errno-only checks for most functions.

It makes me wonder if the recent getdelim() fix is technically violating this. It should instead be explicitly checking for feof().

Show 5 quoted lines
> IMHO as long as it _is_ deterministic and recognize as not an error from
> read(), that's the best we can do. Which is why I went with "0" in the
> first place. Seeing "read error: success" is a common-ish idiom (to me
> anyway) for "read didn't fail, but some user-space logic did", if only
> because it often happens accidentally.
Another option I ran across from POSIX:
  [EOVERFLOW]
    The file is a regular file, nbyte is greater than 0, the starting
    position is before the end-of-file, and the starting position is
    greater than or equal to the offset maximum established in the open
    file description associated with fildes.

That's not _exactly_ what's going on here, but it's pretty close. And is what you would get if you implemented read_exactly() in terms of something like pread().

-Peff
Previous: Jeff KingNext: Jonathan Nieder
Message 12 of 51 in “read/write_in_full leftovers”
  1. 0/7 read/write_in_full leftoversJeff King, Sep 25, 2017
  2. 1/7 files-backend: prefer "0" for write_in_full() error checkJeff King, Sep 25, 2017
  3. Jonathan NiederSep 25, 2017
  4. Jeff KingSep 25, 2017
  5. 2/7 notes-merge: drop dead zero-write codeJeff King, Sep 25, 2017
  6. Jonathan NiederSep 25, 2017
  7. 3/7 read_in_full: reset errno before readingJeff King, Sep 25, 2017
  8. Jonathan NiederSep 25, 2017
  9. Jeff KingSep 25, 2017
  10. Jonathan NiederSep 25, 2017
  11. Jeff KingSep 25, 2017
  12. Jeff KingSep 25, 2017
  13. Jonathan NiederSep 25, 2017
  14. Jeff KingSep 26, 2017
  15. Stefan BellerSep 26, 2017
  16. Jeff KingSep 26, 2017
  17. Jonathan NiederSep 26, 2017
  18. Jonathan NiederSep 26, 2017
  19. Jeff KingSep 26, 2017
  20. Jonathan NiederSep 26, 2017
  21. Junio C HamanoSep 26, 2017
  22. Jeff KingSep 27, 2017
  23. Junio C HamanoSep 27, 2017
  24. Jonathan NiederSep 26, 2017
  25. Jeff KingSep 26, 2017
  26. Jonathan NiederSep 26, 2017
  27. Jeff KingSep 26, 2017
  28. Jonathan NiederSep 26, 2017
  29. Jonathan NiederSep 25, 2017
  30. 4/7 get-tar-commit-id: prefer "!=" for read_in_full() error checkJeff King, Sep 25, 2017
  31. Jonathan NiederSep 25, 2017
  32. 5/7 worktree: use xsize_t to access file sizeJeff King, Sep 25, 2017
  33. Jonathan NiederSep 25, 2017
  34. 6/7 worktree: check the result of read_in_full()Jeff King, Sep 25, 2017
  35. Jonathan NiederSep 25, 2017
  36. Jeff KingSep 25, 2017
  37. 7/7 add xread_in_full() helperJeff King, Sep 25, 2017
  38. Jonathan NiederSep 25, 2017
  39. Jeff KingSep 25, 2017
  40. 0/7 read/write_in_full leftoversJeff King, Sep 27, 2017
  41. 1/7 files-backend: prefer "0" for write_in_full() error checkJeff King, Sep 27, 2017
  42. 2/7 notes-merge: drop dead zero-write codeJeff King, Sep 27, 2017
  43. 3/7 prefer "!=" when checking read_in_full() resultJeff King, Sep 27, 2017
  44. Junio C HamanoSep 27, 2017
  45. Jeff KingSep 27, 2017
  46. 4/7 avoid looking at errno for short read_in_full() returnsJeff King, Sep 27, 2017
  47. Junio C HamanoSep 27, 2017
  48. 5/7 distinguish error versus short read from read_in_full()Jeff King, Sep 27, 2017
  49. 6/7 worktree: use xsize_t to access file sizeJeff King, Sep 27, 2017
  50. 7/7 worktree: check the result of read_in_full()Jeff King, Sep 27, 2017
  51. Junio C HamanoSep 27, 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.