From: Eric Wong <e@80x24.org>
To: Johannes Schindelin <johannes.schindelin@gmx.de>
Cc: git@vger.kernel.org, Ben Wijen <ben@wijen.net>,
Junio C Hamano <gitster@pobox.com>,
Lars Schneider <larsxschneider@gmail.com>
Subject: Re: [PATCH 2/2] mingw: ensure temporary file handles are not inherited by child processes
Date: Thu, 18 Aug 2016 17:35:55 +0000 [thread overview]
Message-ID: <20160818173555.GA29253@starla> (raw)
In-Reply-To: <77e7c4e4de6c45a1b24bb4d08ca20a1385f43444.1471437637.git.johannes.schindelin@gmx.de>
Johannes Schindelin <johannes.schindelin@gmx.de> wrote:
> +++ b/compat/mingw.h
> @@ -67,6 +67,10 @@ typedef int pid_t;
> #define F_SETFD 2
> #define FD_CLOEXEC 0x1
>
> +#if !defined O_CLOEXEC && defined O_NOINHERIT
> +#define O_CLOEXEC O_NOINHERIT
> +#endif
> +++ b/tempfile.c
> @@ -120,7 +120,7 @@ int create_tempfile(struct tempfile *tempfile, const char *path)
> prepare_tempfile_object(tempfile);
>
> strbuf_add_absolute_path(&tempfile->filename, path);
> - tempfile->fd = open(tempfile->filename.buf, O_RDWR | O_CREAT | O_EXCL, 0666);
> + tempfile->fd = open(tempfile->filename.buf, O_RDWR | O_CREAT | O_EXCL | O_CLOEXEC, 0666);
> if (tempfile->fd < 0) {
> strbuf_reset(&tempfile->filename);
> return -1;
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);
if (fd < 0 && errno == EINVAL && cloexec_state == -1 &&
GIT_O_ETMP != GIT_O_TMP) {
GIT_O_ETMP = GIT_O_TMP;
fd = open(filename, GIT_O_ETMP, 0666);
if (fd >= 0)
/* don't try O_CLOEXEC again */
cloexec_state = 0;
}
/*
* 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
...
next prev parent reply other threads:[~2016-08-19 1:21 UTC|newest]
Thread overview: 21+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-08-17 12:40 [PATCH 0/2] Do not lock temporary files via child processes on Windows Johannes Schindelin
2016-08-17 12:40 ` [PATCH 1/2] t6026-merge-attr: child processes must not inherit index.lock handles Johannes Schindelin
2016-08-17 17:55 ` Junio C Hamano
2016-08-17 12:41 ` [PATCH 2/2] mingw: ensure temporary file handles are not inherited by child processes Johannes Schindelin
2016-08-17 12:48 ` Eric Sunshine
2016-08-18 14:42 ` Johannes Schindelin
2016-08-17 13:14 ` Lars Schneider
2016-08-17 18:22 ` Junio C Hamano
2016-08-18 14:50 ` Johannes Schindelin
2016-08-18 17:35 ` Eric Wong [this message]
2016-08-18 21:53 ` Junio C Hamano
2016-08-18 22:48 ` Eric Wong
2016-08-19 15:57 ` Junio C Hamano
2016-08-22 12:47 ` Johannes Schindelin
2016-08-18 14:51 ` [PATCH v2 0/2] Do not lock temporary files via child processes on Windows Johannes Schindelin
2016-08-18 14:51 ` [PATCH v2 1/2] t6026-merge-attr: child processes must not inherit index.lock handles Johannes Schindelin
2016-08-18 14:51 ` [PATCH v2 2/2] mingw: ensure temporary file handles are not inherited by child processes Johannes Schindelin
2016-08-22 12:47 ` [PATCH v3 0/2] Do not lock temporary files via child processes on Windows Johannes Schindelin
2016-08-22 12:47 ` [PATCH v3 1/2] t6026-merge-attr: child processes must not inherit index.lock handles Johannes Schindelin
2016-08-22 12:47 ` [PATCH v3 2/2] mingw: ensure temporary file handles are not inherited by child processes Johannes Schindelin
2016-08-22 17:58 ` Eric Wong
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20160818173555.GA29253@starla \
--to=e@80x24.org \
--cc=ben@wijen.net \
--cc=git@vger.kernel.org \
--cc=gitster@pobox.com \
--cc=johannes.schindelin@gmx.de \
--cc=larsxschneider@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.