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

Re: [PATCH 2/2] mingw: ensure temporary file handles are not inherited by child processes

From
Junio C Hamano <gitster@pobox.com>
Date
Aug 18, 2016, 21:53 UTC
Message-ID
<xmqqshu14udc.fsf@gitster.mtv.corp.google.com>
In-Reply-To
<20160818173555.GA29253@starla>
Eric Wong <e@80x24.org> writes:
Show 40 quoted lines
> O_CLOEXEC only exists since Linux 2.6.23 and there are likely
> still LTS (CentOS 5.x?) and non-Linux systems which do not have
> it, as well as machines with could have it defined in userspace
> headers but not have it in the kernel.
>
> So I suggest something like the following: (untested)
>
> #define GIT_O_TMP (O_RDWR | O_CREAT | O_EXCL)
>
> #ifndef O_CLOEXEC
> #  define O_CLOEXEC 0
> #endif
> 	/* state: -1=unknown; 0=broken; 1=working */
> 	static int cloexec_state = O_CLOEXEC == 0 ? 0 : -1;
> 	static int GIT_O_ETMP = (GIT_O_TMP | O_CLOEXEC)
>
> 	int fd = open(filename, GIT_O_ETMP, 0666);
> ...
> 	/*
> 	 * This is racy in the presence of threads,
> 	 * but the best we can do for old *nix:
> 	 */
> #if defined(F_GETFD) && defined(F_SETFD) && defined(FD_CLOEXEC)
> 	if (fd >= 0 && cloexec_state != 1) {
> 		int flags = fcntl(fd, F_GETFD);
>
> 		if (flags == -1)
> 			die_errno("F_GETFD failed");
> 		if (flags & O_CLOEXEC)
> 			cloexec_state = 1;
> 		else {
> 			flags = fcntl(fd, F_SETFD, flags | FD_CLOEXEC);
> 			if (flags == -1)
> 				die_errno("F_SETFD failed");
> 			cloexec_state = 0;
>
> 		}
> 	}
> #endif
> 	...

Our codepaths themselves generally do not care about O_CLOEXEC, so giving a racy emulation of it is not worth the effort, making the later half of the above an overkill. Perhaps the three lines to define O_CLOEXEC to 0 on older UNIX might be sufficient.

Previous: Eric WongNext: Eric Wong
Message 11 of 21 in “Do not lock temporary files via child processes on Windows”
  1. 0/2 Do not lock temporary files via child processes on WindowsJohannes Schindelin, Aug 17, 2016
  2. 1/2 t6026-merge-attr: child processes must not inherit index.lock handlesJohannes Schindelin, Aug 17, 2016
  3. Junio C HamanoAug 17, 2016
  4. 2/2 mingw: ensure temporary file handles are not inherited by child processesJohannes Schindelin, Aug 17, 2016
  5. Eric SunshineAug 17, 2016
  6. Johannes SchindelinAug 18, 2016
  7. Lars SchneiderAug 17, 2016
  8. Junio C HamanoAug 17, 2016
  9. Johannes SchindelinAug 18, 2016
  10. Eric WongAug 18, 2016
  11. Junio C HamanoAug 18, 2016
  12. Eric WongAug 18, 2016
  13. Junio C HamanoAug 19, 2016
  14. Johannes SchindelinAug 22, 2016
  15. 0/2 Do not lock temporary files via child processes on WindowsJohannes Schindelin, Aug 18, 2016
  16. 2/2 mingw: ensure temporary file handles are not inherited by child processesJohannes Schindelin, Aug 18, 2016
  17. 1/2 t6026-merge-attr: child processes must not inherit index.lock handlesJohannes Schindelin, Aug 18, 2016
  18. 0/2 Do not lock temporary files via child processes on WindowsJohannes Schindelin, Aug 22, 2016
  19. 1/2 t6026-merge-attr: child processes must not inherit index.lock handlesJohannes Schindelin, Aug 22, 2016
  20. 2/2 mingw: ensure temporary file handles are not inherited by child processesJohannes Schindelin, Aug 22, 2016
  21. Eric WongAug 22, 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.