[Patch, libgfortran] Set close-on-exec flag when opening files

[Janne Blomqvist]

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
-------------- next part --------------
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)