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

Re: [PATCH v3 2/3] sha1_file: open window into packfiles with O_CLOEXEC

From
Johannes Schindelin <johannes.schindelin@gmx.de>
Date
Oct 28, 2016, 11:11 UTC
Message-ID
<alpine.DEB.2.20.1610281306320.3264@virtualbox>
In-Reply-To
<xmqq60od42s0.fsf@gitster.mtv.corp.google.com>
Hi,
On Thu, 27 Oct 2016, Junio C Hamano wrote:
Show 27 quoted lines
> Junio C Hamano <gitster@pobox.com> writes:
> 
> > Linus Torvalds <torvalds@linux-foundation.org> writes:
> >
> >> On Thu, Oct 27, 2016 at 4:36 PM, Junio C Hamano <gitster@pobox.com> wrote:
> >>>
> >>> Would the best endgame shape for this function be to open with
> >>> O_NOATIME (and retry without), and then add CLOEXEC with fcntl(2)
> >>> but ignoring an error from it, I guess?  That would be the closest
> >>> to what we historically had, I would think.
> >>
> >> I think that's the best model.
> >
> > OK, so perhaps like this.
> 
> Hmph.  This may not fly well in practice, though.  
> 
> To Unix folks, CLOEXEC is not a huge correctness issue.  A child
> process may hold onto an open file descriptor a bit longer than the
> lifetime of the parent but as long as the child eventually exits,
> nothing is affected.  Over there, things are different.  The parent
> cannot even rename(2) or unlink(2) a file it created and closed
> while the child is still holding the file descriptor open and the
> lack of CLOEXEC will make the parent fail.  I do not know how well
> fcntl(2) emulation works on Windows, but I would not be surprised
> if J6t or Dscho comes back and says that FD_CLOEXEC given to F_SETFD
> would not work while O_CLOEXEC given to open(2) does.

You guys. I mean: You guys! You sure make my life hard. A brief look at mingw.h could have answered your implicit question:

	static inline int fcntl(int fd, int cmd, ...)
	{
		if (cmd == F_GETFD || cmd == F_SETFD)
			return 0;
		errno = EINVAL;
		return -1;
	}

So while you discuss in your Linux Ivory Tower how to optimize Git for Linux, and Linux only, I'll have to drop everything else and spend the rest of my Friday trying to find a way to adjust a file handle *immediately after opening it with undesired flags* (when it could have been opened with the desired flags, as suggested, to begin with).

Ciao, Johannes

Previous: Eric WongNext: Linus Torvalds
Message 34 of 54 in “Use CLOEXEC to avoid fd leaks”
  1. 0/2 Use CLOEXEC to avoid fd leakslarsxschneider@gmail.com, Oct 24, 2016
  2. 1/2 sha1_file: open window into packfiles with CLOEXEClarsxschneider@gmail.com, Oct 24, 2016
  3. Johannes SchindelinOct 25, 2016
  4. Junio C HamanoOct 25, 2016
  5. 2/2 read-cache: make sure file handles are not inherited by child processeslarsxschneider@gmail.com, Oct 24, 2016
  6. Eric WongOct 24, 2016
  7. Junio C HamanoOct 24, 2016
  8. Johannes SchindelinOct 25, 2016
  9. Junio C HamanoOct 25, 2016
  10. Johannes SixtOct 24, 2016
  11. Lars SchneiderOct 24, 2016
  12. Johannes SixtOct 25, 2016
  13. Junio C HamanoOct 24, 2016
  14. Johannes SchindelinOct 25, 2016
  15. 0/3 quick reroll of Lars's git_open() w/ O_CLOEXECJunio C Hamano, Oct 25, 2016
  16. 2/3 sha1_file: open window into packfiles with O_CLOEXECJunio C Hamano, Oct 25, 2016
  17. Jeff KingOct 26, 2016
  18. Junio C HamanoOct 26, 2016
  19. Jeff KingOct 26, 2016
  20. Junio C HamanoOct 26, 2016
  21. Jeff KingOct 26, 2016
  22. Junio C HamanoOct 26, 2016
  23. Jeff KingOct 27, 2016
  24. Junio C HamanoOct 27, 2016
  25. Linus TorvaldsOct 27, 2016
  26. Junio C HamanoOct 27, 2016
  27. Linus TorvaldsOct 27, 2016
  28. Linus TorvaldsOct 27, 2016
  29. Junio C HamanoOct 27, 2016
  30. Linus TorvaldsOct 27, 2016
  31. Junio C HamanoOct 28, 2016
  32. Junio C HamanoOct 28, 2016
  33. Eric WongOct 28, 2016
  34. Johannes SchindelinOct 28, 2016
  35. Linus TorvaldsOct 28, 2016
  36. Junio C HamanoOct 28, 2016
  37. Linus TorvaldsOct 28, 2016
  38. Junio C HamanoOct 28, 2016
  39. Junio C HamanoOct 29, 2016
  40. Johannes SchindelinOct 29, 2016
  41. Linus TorvaldsOct 29, 2016
  42. Junio C HamanoOct 31, 2016
  43. Jeff KingOct 31, 2016
  44. Junio C HamanoOct 31, 2016
  45. Jeff KingOct 31, 2016
  46. Junio C HamanoOct 28, 2016
  47. Junio C HamanoOct 28, 2016
  48. Jeff KingOct 28, 2016
  49. 1/3 sha1_file: rename git_open_noatime() to git_open()Junio C Hamano, Oct 25, 2016
  50. 3/3 read-cache: make sure file handles are not inherited by child processesJunio C Hamano, Oct 25, 2016
  51. Eric WongOct 25, 2016
  52. Junio C HamanoOct 25, 2016
  53. Lars SchneiderOct 25, 2016
  54. Junio C HamanoOct 25, 2016

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.