On Sun, Nov 10, 2013 at 7:16 PM, Tobias Burnus <bur...@net-b.de> wrote:
> Janne Blomqvist wrote:
>>
>> the attached patch sets the close-on-exec flag when opening files, as
>> is usually considered good practice these days. See e.g.
>> http://www.python.org/dev/peps/pep-0446/ and links therein for more
>> information.
>
>
>> + int flags = O_RDWR|O_CREAT|O_EXCL;
>
> I'd add spaces around "|".
>
>
> Otherwise, it looks good to me. Thanks for the patch and sorry for the slow
> review.
Thanks for the review, committed as r204654. The committed patch
differs from the one sent for review in the following aspects:
- Fixed the issue you mentioned above
- Fixed another "spaces around |" issue
- In set_close_on_exec(), call fcntl only if fd >= 0; this prevents
clobbering errno if something went wrong earlier
- Add __attribute__((unused)) to set_close_on_exec, preventing a
warning on systems having both O_CLOEXEC and mkostemp() (such as
recent Linux)
Committed patch attached. I'll add a note to the wiki as well, to be
added to the release notes at some later point by someone.
--
Janne Blomqvist
diff --git a/libgfortran/configure.ac b/libgfortran/configure.ac
index 4609eba..6417373 100644
--- a/libgfortran/configure.ac
+++ b/libgfortran/configure.ac
@@ -280,7 +280,7 @@ else
strcasestr getrlimit gettimeofday stat fstat lstat getpwuid vsnprintf dup \
getcwd localtime_r gmtime_r getpwuid_r ttyname_r clock_gettime \
readlink getgid getpid getppid getuid geteuid umask getegid \
- secure_getenv __secure_getenv)
+ secure_getenv __secure_getenv mkostemp)
fi
# Check strerror_r, cannot be above as versions with two and three arguments
exist
diff --git a/libgfortran/io/unix.c b/libgfortran/io/unix.c
index dd2715b..8a84ae4 100644
--- a/libgfortran/io/unix.c
+++ b/libgfortran/io/unix.c
@@ -1070,6 +1070,20 @@ unpack_filename (char *cstring, const char *fstring, int
len)
}
+/* Set the close-on-exec flag for an existing fd, if the system
+ supports such. */
+
+static void __attribute__ ((unused))
+set_close_on_exec (int fd __attribute__ ((unused)))
+{
+ /* Mingw does not define F_SETFD. */
+#if defined(F_SETFD) && defined(FD_CLOEXEC)
+ if (fd >= 0)
+ fcntl(fd, F_SETFD, FD_CLOEXEC);
+#endif
+}
+
+
/* Helper function for tempfile(). Tries to open a temporary file in
the directory specified by tempdir. If successful, the file name is
stored in fname and the descriptor returned. Returns -1 on
@@ -1109,7 +1123,12 @@ tempfile_open (const char *tempdir, char **fname)
mode_mask = umask (S_IXUSR | S_IRWXG | S_IRWXO);
#endif
+#if defined(HAVE_MKOSTEMP) && defined(O_CLOEXEC)
+ fd = mkostemp (template, O_RDWR | O_CREAT | O_EXCL | O_CLOEXEC);
+#else
fd = mkstemp (template);
+ set_close_on_exec (fd);
+#endif
#ifdef HAVE_UMASK
(void) umask (mode_mask);
@@ -1119,6 +1138,13 @@ tempfile_open (const char *tempdir, char **fname)
fd = -1;
int count = 0;
size_t slashlen = strlen (slash);
+ int flags = O_RDWR | O_CREAT | O_EXCL;
+#if defined(HAVE_CRLF) && defined(O_BINARY)
+ flags |= O_BINARY;
+#endif
+#ifdef O_CLOEXEC
+ flags |= O_CLOEXEC;
+#endif
do
{
snprintf (template, tempdirlen + 23, "%s%sgfortrantmpaaaXXXXXX",
@@ -1142,14 +1168,12 @@ tempfile_open (const char *tempdir, char **fname)
continue;
}
-#if defined(HAVE_CRLF) && defined(O_BINARY)
- fd = open (template, O_RDWR | O_CREAT | O_EXCL | O_BINARY,
- S_IRUSR | S_IWUSR);
-#else
- fd = open (template, O_RDWR | O_CREAT | O_EXCL, S_IRUSR | S_IWUSR);
-#endif
+ fd = open (template, flags, S_IRUSR | S_IWUSR);
}
while (fd == -1 && errno == EEXIST);
+#ifndef O_CLOEXEC
+ set_close_on_exec (fd);
+#endif
#endif /* HAVE_MKSTEMP */
*fname = template;
@@ -1323,6 +1347,10 @@ regular_file (st_parameter_open *opp, unit_flags *flags)
crflag |= O_BINARY;
#endif
+#ifdef O_CLOEXEC
+ crflag |= O_CLOEXEC;
+#endif
+
mode = S_IRUSR | S_IWUSR | S_IRGRP | S_IWGRP | S_IROTH | S_IWOTH;
fd = open (path, rwflag | crflag, mode);
if (flags->action != ACTION_UNSPECIFIED)
@@ -1386,6 +1414,9 @@ open_external (st_parameter_open *opp, unit_flags *flags)
/* regular_file resets flags->action if it is ACTION_UNSPECIFIED and
* if it succeeds */
fd = regular_file (opp, flags);
+#ifndef O_CLOEXEC
+ set_close_on_exec (fd);
+#endif
}
if (fd < 0)
