* [Patch, libgfortran] Set close-on-exec flag when opening files
@ 2013-10-30 0:09 Janne Blomqvist
2013-11-05 22:55 ` Janne Blomqvist
` (2 more replies)
0 siblings, 3 replies; 5+ messages in thread
From: Janne Blomqvist @ 2013-10-30 0:09 UTC (permalink / raw)
To: Fortran List, GCC Patches
[-- Attachment #1: Type: text/plain, Size: 1073 bytes --]
Hello,
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.
The preconnected units INPUT_UNIT, OUTPUT_UNIT, ERROR_UNIT are not
affected, only new units created with the OPEN statement. In the
(very!?) unlikely event that someone really needs Fortran units to be
inherited to child processes, the close-on-exec flag can be cleared
with fcntl() before calling one of the exec() family functions.
Regtested on x86_64-unknown-linux-gnu, Ok for trunk?
2013-10-30 Janne Blomqvist <jb@gcc.gnu.org>
* configure.ac: Check presence of mkostemp.
* io/unix.c (set_close_on_exec): New function.
(tempfile_open): Use mkostemp and O_CLOEXEC if available, fallback
to calling set_close_on_exec.
(regular_file): Add O_CLOEXEC to flags if defined.
(open_external): Call set_close_on_exec if O_CLOEXEC is not
defined.
* config.h.in: Regenerated.
* configure: Regenerated.
--
Janne Blomqvist
[-- Attachment #2: cloexec.diff --]
[-- Type: text/x-patch, Size: 3214 bytes --]
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..ae48d57 100644
--- a/libgfortran/io/unix.c
+++ b/libgfortran/io/unix.c
@@ -1070,6 +1070,19 @@ 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
+set_close_on_exec (int fd __attribute__ ((unused)))
+{
+ /* Mingw does not define F_SETFD. */
+#if defined(F_SETFD) && defined(FD_CLOEXEC)
+ 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 +1122,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 +1137,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 +1167,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 +1346,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 +1413,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)
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [Patch, libgfortran] Set close-on-exec flag when opening files
2013-10-30 0:09 [Patch, libgfortran] Set close-on-exec flag when opening files Janne Blomqvist
@ 2013-11-05 22:55 ` Janne Blomqvist
2013-11-10 18:43 ` Tobias Burnus
2014-03-18 9:34 ` [patch, libgfortran] Fix SPU link failures (Re: Set close-on-exec flag when opening files) Ulrich Weigand
2 siblings, 0 replies; 5+ messages in thread
From: Janne Blomqvist @ 2013-11-05 22:55 UTC (permalink / raw)
To: Fortran List, GCC Patches
PING
On Wed, Oct 30, 2013 at 1:39 AM, Janne Blomqvist
<blomqvist.janne@gmail.com> wrote:
> Hello,
>
> 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.
>
> The preconnected units INPUT_UNIT, OUTPUT_UNIT, ERROR_UNIT are not
> affected, only new units created with the OPEN statement. In the
> (very!?) unlikely event that someone really needs Fortran units to be
> inherited to child processes, the close-on-exec flag can be cleared
> with fcntl() before calling one of the exec() family functions.
>
> Regtested on x86_64-unknown-linux-gnu, Ok for trunk?
>
> 2013-10-30 Janne Blomqvist <jb@gcc.gnu.org>
>
> * configure.ac: Check presence of mkostemp.
> * io/unix.c (set_close_on_exec): New function.
> (tempfile_open): Use mkostemp and O_CLOEXEC if available, fallback
> to calling set_close_on_exec.
> (regular_file): Add O_CLOEXEC to flags if defined.
> (open_external): Call set_close_on_exec if O_CLOEXEC is not
> defined.
> * config.h.in: Regenerated.
> * configure: Regenerated.
>
>
> --
> Janne Blomqvist
--
Janne Blomqvist
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [Patch, libgfortran] Set close-on-exec flag when opening files
2013-10-30 0:09 [Patch, libgfortran] Set close-on-exec flag when opening files Janne Blomqvist
2013-11-05 22:55 ` Janne Blomqvist
@ 2013-11-10 18:43 ` Tobias Burnus
2013-11-10 23:21 ` Janne Blomqvist
2014-03-18 9:34 ` [patch, libgfortran] Fix SPU link failures (Re: Set close-on-exec flag when opening files) Ulrich Weigand
2 siblings, 1 reply; 5+ messages in thread
From: Tobias Burnus @ 2013-11-10 18:43 UTC (permalink / raw)
To: Janne Blomqvist, Fortran List, GCC Patches
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.
Tobias
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [Patch, libgfortran] Set close-on-exec flag when opening files
2013-11-10 18:43 ` Tobias Burnus
@ 2013-11-10 23:21 ` Janne Blomqvist
0 siblings, 0 replies; 5+ messages in thread
From: Janne Blomqvist @ 2013-11-10 23:21 UTC (permalink / raw)
To: Tobias Burnus; +Cc: Fortran List, GCC Patches
[-- Attachment #1: Type: text/plain, Size: 1108 bytes --]
On Sun, Nov 10, 2013 at 7:16 PM, Tobias Burnus <burnus@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
[-- Attachment #2: cloexec2.diff --]
[-- Type: text/plain, Size: 3267 bytes --]
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)
^ permalink raw reply [flat|nested] 5+ messages in thread
* [patch, libgfortran] Fix SPU link failures (Re: Set close-on-exec flag when opening files)
2013-10-30 0:09 [Patch, libgfortran] Set close-on-exec flag when opening files Janne Blomqvist
2013-11-05 22:55 ` Janne Blomqvist
2013-11-10 18:43 ` Tobias Burnus
@ 2014-03-18 9:34 ` Ulrich Weigand
2 siblings, 0 replies; 5+ messages in thread
From: Ulrich Weigand @ 2014-03-18 9:34 UTC (permalink / raw)
To: fortran, gcc-patches; +Cc: blomqvist.janne
Janne Blomqvist wrote:
> * io/unix.c (set_close_on_exec): New function.
Since this patch, most Fortran tests fail on spu-elf since the system
libraries do not support fcntl on the SPU.
The patch below fixes this by using an autoconf check to verify
fcntl is present before using it, as is already done for many
other routines in io/unix.c.
Tested on spu-elf, fixes the Fortran problems there.
Also verified on an older powerpc64-linux build that we still
use fcntl there.
OK for mainline?
Bye,
Ulrich
ChangeLog:
* configure.ac: Check for presence of fcntl.
* configure: Regenerate.
* config.h.in: Regenerate.
* io/unix.c (set_close_on_exec): Check for HAVE_FCNTL.
Index: libgfortran/io/unix.c
===================================================================
*** libgfortran/io/unix.c (revision 208613)
--- libgfortran/io/unix.c (working copy)
*************** static void __attribute__ ((unused))
*** 1077,1083 ****
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
--- 1077,1083 ----
set_close_on_exec (int fd __attribute__ ((unused)))
{
/* Mingw does not define F_SETFD. */
! #if defined(HAVE_FCNTL) && defined(F_SETFD) && defined(FD_CLOEXEC)
if (fd >= 0)
fcntl(fd, F_SETFD, FD_CLOEXEC);
#endif
Index: libgfortran/configure.ac
===================================================================
*** libgfortran/configure.ac (revision 208613)
--- libgfortran/configure.ac (working copy)
*************** if test "x${with_newlib}" = "xyes"; then
*** 282,288 ****
else
AC_CHECK_FUNCS_ONCE(getrusage times mkstemp strtof strtold snprintf \
ftruncate chsize chdir getlogin gethostname kill link symlink sleep ttyname \
! alarm access fork execl wait setmode execve pipe dup2 close \
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 \
--- 282,288 ----
else
AC_CHECK_FUNCS_ONCE(getrusage times mkstemp strtof strtold snprintf \
ftruncate chsize chdir getlogin gethostname kill link symlink sleep ttyname \
! alarm access fork execl wait setmode execve pipe dup2 close fcntl \
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 \
Index: libgfortran/config.h.in
===================================================================
*** libgfortran/config.h.in (revision 208613)
--- libgfortran/config.h.in (working copy)
***************
*** 360,365 ****
--- 360,368 ----
/* Define to 1 if you have the `fabsl' function. */
#undef HAVE_FABSL
+ /* Define to 1 if you have the `fcntl' function. */
+ #undef HAVE_FCNTL
+
/* libm includes feenableexcept */
#undef HAVE_FEENABLEEXCEPT
Index: libgfortran/configure
===================================================================
*** libgfortran/configure (revision 208613)
--- libgfortran/configure (working copy)
*************** as_fn_append ac_func_list " execve"
*** 2572,2577 ****
--- 2572,2578 ----
as_fn_append ac_func_list " pipe"
as_fn_append ac_func_list " dup2"
as_fn_append ac_func_list " close"
+ as_fn_append ac_func_list " fcntl"
as_fn_append ac_func_list " strcasestr"
as_fn_append ac_func_list " getrlimit"
as_fn_append ac_func_list " gettimeofday"
*************** else
*** 12342,12348 ****
lt_dlunknown=0; lt_dlno_uscore=1; lt_dlneed_uscore=2
lt_status=$lt_dlunknown
cat > conftest.$ac_ext <<_LT_EOF
! #line 12345 "configure"
#include "confdefs.h"
#if HAVE_DLFCN_H
--- 12343,12349 ----
lt_dlunknown=0; lt_dlno_uscore=1; lt_dlneed_uscore=2
lt_status=$lt_dlunknown
cat > conftest.$ac_ext <<_LT_EOF
! #line 12346 "configure"
#include "confdefs.h"
#if HAVE_DLFCN_H
*************** else
*** 12448,12454 ****
lt_dlunknown=0; lt_dlno_uscore=1; lt_dlneed_uscore=2
lt_status=$lt_dlunknown
cat > conftest.$ac_ext <<_LT_EOF
! #line 12451 "configure"
#include "confdefs.h"
#if HAVE_DLFCN_H
--- 12449,12455 ----
lt_dlunknown=0; lt_dlno_uscore=1; lt_dlneed_uscore=2
lt_status=$lt_dlunknown
cat > conftest.$ac_ext <<_LT_EOF
! #line 12452 "configure"
#include "confdefs.h"
#if HAVE_DLFCN_H
*************** done
*** 16602,16607 ****
--- 16603,16610 ----
+
+
fi
# Check strerror_r, cannot be above as versions with two and three arguments exist
--
Dr. Ulrich Weigand
GNU/Linux compilers and toolchain
Ulrich.Weigand@de.ibm.com
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2014-03-18 9:23 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-10-30 0:09 [Patch, libgfortran] Set close-on-exec flag when opening files Janne Blomqvist
2013-11-05 22:55 ` Janne Blomqvist
2013-11-10 18:43 ` Tobias Burnus
2013-11-10 23:21 ` Janne Blomqvist
2014-03-18 9:34 ` [patch, libgfortran] Fix SPU link failures (Re: Set close-on-exec flag when opening files) Ulrich Weigand
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).