Clean up close-on-exec, particularly with jobserver pipes.

* configure.ac: Check sys/file.h and assume fileno() always exists.
* output.h: Move output-specific content from job.h to output.h.
* os.h (fd_inherit, fd_noinherit): New functions manage FD inheritance.
* posixos.c (fd_inherit, fd_noinherit): Implement for POSIX systems.
(jobserver_setup): Force jobserver FDs to not be inherited by default.
(jobserver_pre_child): Enable inheritance in recursive invocations.
(jobserver_post_child): Disable inheritance after recursive invocations.
* w32/w32os.c (fd_inherit, fd_noinherit): Implement for W32 systems.
* job.h (CLOSE_ON_EXEC): Remove macro in deference to new fd_noinherit.
* function.c (func_shell_base): Convert CLOSE_ON_EXEC to fd_noinherit.
* job.c (child_execute_job): Ditto.
* output.c (setup_tmpfile): Ditto.
* read.c (eval_makefile): Ditto, plus remove HAVE_FILENO check.
* w32/include/sub_proc.h: Remove process_noinherit for fd_noinherit.
* w32/subproc/sub_proc.c: Ditto.

diff --git a/configure.ac b/configure.ac
index a78fb932f1df36c3c43177a046891df4c05bcb0e..153a716c38c336d47f9fc10f642aeca9f0c3aa0c 100644 (file)
--- a/configure.ac
+++ b/configure.ac
@@ -69,7 +69,7 @@ AC_HEADER_STAT
 AC_HEADER_TIME
 AC_CHECK_HEADERS([stdlib.h locale.h unistd.h limits.h fcntl.h string.h \
                   memory.h sys/param.h sys/resource.h sys/time.h sys/timeb.h \
-                  sys/select.h])
+                  sys/select.h sys/file.h])
 
 AM_PROG_CC_C_O
 AC_C_CONST
@@ -132,7 +132,7 @@ AS_IF([test "$ac_cv_func_gettimeofday" = yes],
             [Define to 1 if you have a standard gettimeofday function])
 ])
 
-AC_CHECK_FUNCS([strdup strndup umask mkstemp mktemp fdopen fileno \
+AC_CHECK_FUNCS([strdup strndup umask mkstemp mktemp fdopen \
                 dup dup2 getcwd realpath sigsetmask sigaction \
                 getgroups seteuid setegid setlinebuf setreuid setregid \
                 getrlimit setrlimit setvbuf pipe strerror strsignal \

diff --git a/function.c b/function.c
index ebbc88ea0a0a749cc4dee58819d559d2deed67a1..7a2fcd4bc80495c649bd392bf06ff0bee45aa0d2 100644 (file)
--- a/function.c
+++ b/function.c
@@ -19,6 +19,7 @@ this program.  If not, see <http://www.gnu.org/licenses/>.  */
 #include "variable.h"
 #include "dep.h"
 #include "job.h"
+#include "os.h"
 #include "commands.h"
 #include "debug.h"
 
@@ -1778,8 +1779,8 @@ func_shell_base (char *o, char **argv, int trim_newlines)
     }
 
   /* Close handles that are unnecessary for the child process.  */
-  CLOSE_ON_EXEC(pipedes[1]);
-  CLOSE_ON_EXEC(pipedes[0]);
+  fd_noinherit (pipedes[1]);
+  fd_noinherit (pipedes[0]);
 
   {
     struct output out;

diff --git a/job.c b/job.c
index 194d2576d9f77984b10edfe9661a6bf7078cc9a6..e2f2273d3f0da2e1810d8573de6d3a145138d0f2 100644 (file)
--- a/job.c
+++ b/job.c
@@ -2052,10 +2052,10 @@ child_execute_job (struct output *out, int good_stdin, char **argv, char **envp)
       save_fdin = dup (FD_STDIN);
       if (save_fdin < 0)
         O (fatal, NILF, _("no more file handles: could not duplicate stdin\n"));
-      CLOSE_ON_EXEC (save_fdin);
+      fd_noinherit (save_fdin);
 
       dup2 (fdin, FD_STDIN);
-      CLOSE_ON_EXEC (fdin);
+      fd_noinherit (fdin);
     }
 
   if (fdout != FD_STDOUT)
@@ -2064,10 +2064,10 @@ child_execute_job (struct output *out, int good_stdin, char **argv, char **envp)
       if (save_fdout < 0)
         O (fatal, NILF,
            _("no more file handles: could not duplicate stdout\n"));
-      CLOSE_ON_EXEC (save_fdout);
+      fd_noinherit (save_fdout);
 
       dup2 (fdout, FD_STDOUT);
-      CLOSE_ON_EXEC (fdout);
+      fd_noinherit (fdout);
     }
 
   if (fderr != FD_STDERR)
@@ -2078,11 +2078,11 @@ child_execute_job (struct output *out, int good_stdin, char **argv, char **envp)
           if (save_fderr < 0)
             O (fatal, NILF,
                _("no more file handles: could not duplicate stderr\n"));
-          CLOSE_ON_EXEC (save_fderr);
+          fd_noinherit (save_fderr);
         }
 
       dup2 (fderr, FD_STDERR);
-      CLOSE_ON_EXEC (fderr);
+      fd_noinherit (fderr);
     }
 
   /* Run the command.  */

diff --git a/job.h b/job.h
index 37cceb6d089cf4a635a3ada9c02687ca0ad733ef..48cce764faa4a609430523e404345ed510444b38 100644 (file)
--- a/job.h
+++ b/job.h
@@ -16,72 +16,6 @@ this program.  If not, see <http://www.gnu.org/licenses/>.  */
 
 #include "output.h"
 
-#ifdef HAVE_FCNTL_H
-# include <fcntl.h>
-#else
-# include <sys/file.h>
-#endif
-
-/* How to set close-on-exec for a file descriptor.  */
-
-#if !defined(F_SETFD) || !defined(F_GETFD)
-# ifdef WINDOWS32
-#  define CLOSE_ON_EXEC(_d)  process_noinherit(_d)
-# else
-#  define CLOSE_ON_EXEC(_d)
-# endif
-#else
-# ifndef FD_CLOEXEC
-#  define FD_CLOEXEC 1
-# endif
-# define CLOSE_ON_EXEC(_d) (void) fcntl ((_d), F_SETFD, FD_CLOEXEC)
-#endif
-
-#ifdef NO_OUTPUT_SYNC
-# define RECORD_SYNC_MUTEX(m) \
-    O (error, NILF,                                                    \
-       _("-O[TYPE] (--output-sync[=TYPE]) is not configured for this build."));
-#else
-# ifdef WINDOWS32
-/* For emulations in w32/compat/posixfcn.c.  */
-#  define F_GETFD 1
-#  define F_SETLKW 2
-/* Implementation note: None of the values of l_type below can be zero
-   -- they are compared with a static instance of the struct, so zero
-   means unknown/invalid, see w32/compat/posixfcn.c. */
-#  define F_WRLCK 1
-#  define F_UNLCK 2
-
-struct flock
-  {
-    short l_type;
-    short l_whence;
-    off_t l_start;
-    off_t l_len;
-    pid_t l_pid;
-  };
-
-/* This type is actually a HANDLE, but we want to avoid including
-   windows.h as much as possible.  */
-typedef intptr_t sync_handle_t;
-
-/* Public functions emulated/provided in posixfcn.c.  */
-int fcntl (intptr_t fd, int cmd, ...);
-intptr_t create_mutex (void);
-int same_stream (FILE *f1, FILE *f2);
-
-#  define RECORD_SYNC_MUTEX(m) record_sync_mutex(m)
-void record_sync_mutex (const char *str);
-void prepare_mutex_handle_string (intptr_t hdl);
-# else  /* !WINDOWS32 */
-
-typedef int sync_handle_t;      /* file descriptor */
-
-#  define RECORD_SYNC_MUTEX(m) (void)(m)
-
-# endif
-#endif  /* !NO_OUTPUT_SYNC */
-
 /* Structure describing a running or dead child process.  */
 
 struct child

diff --git a/os.h b/os.h
index c1a19e1b76ee98758eb7073a6a69b46cec60667e..111fa9cbea462ddc7680e1f03d82649a22c1de53 100644 (file)
--- a/os.h
+++ b/os.h
@@ -77,8 +77,17 @@ unsigned int jobserver_acquire (int timeout);
 #endif
 
 /* Create a "bad" file descriptor for stdin when parallel jobs are run.  */
-#if !defined(VMD) && !defined(WINDOWS32) && !defined(_AMIGA) && !defined(__MSDOS__)
+#if defined(VMS) || defined(WINDOWS32) || defined(_AMIGA) || defined(__MSDOS__)
+# define get_bad_stdin() (-1)
+#else
 int get_bad_stdin (void);
+#endif
+
+/* Set a file descriptor to close/not close in a subprocess.  */
+#if defined(VMS) || defined(_AMIGA) || defined(__MSDOS__)
+# define fd_inherit(_i)   0
+# define fd_noinherit(_i) 0
 #else
-# define get_bad_stdin() (-1)
+void fd_inherit (int);
+void fd_noinherit (int);
 #endif

diff --git a/output.c b/output.c
index 89bb6d14b5e33391e5ab6545111abe18d2cda3bd..b6ba3cfce1765448016cc77708522fcfe7c40ca8 100644 (file)
--- a/output.c
+++ b/output.c
@@ -15,7 +15,8 @@ You should have received a copy of the GNU General Public License along with
 this program.  If not, see <http://www.gnu.org/licenses/>.  */
 
 #include "makeint.h"
-#include "job.h"
+#include "os.h"
+#include "output.h"
 
 /* GNU make no longer supports pre-ANSI89 environments.  */
 
@@ -335,7 +336,7 @@ setup_tmpfile (struct output *out)
       int fd = output_tmpfd ();
       if (fd < 0)
         goto error;
-      CLOSE_ON_EXEC (fd);
+      fd_noinherit (fd);
       out->out = fd;
     }
 
@@ -348,7 +349,7 @@ setup_tmpfile (struct output *out)
           int fd = output_tmpfd ();
           if (fd < 0)
             goto error;
-          CLOSE_ON_EXEC (fd);
+          fd_noinherit (fd);
           out->err = fd;
         }
     }

diff --git a/output.h b/output.h
index f4fe0653ae9ca9d8be72cc1d292f7a8e2d9083ff..bcd98051e986ddbef68bafb6bbdecaa5dbf4db69 100644 (file)
--- a/output.h
+++ b/output.h
@@ -44,8 +44,57 @@ void output_start (void);
 /* Show a message on stdout or stderr.  Will start the output if needed.  */
 void outputs (int is_err, const char *msg);
 
-#ifndef NO_OUTPUT_SYNC
+#if defined(HAVE_FCNTL_H)
+# include <fcntl.h>
+#elif defined(HAVE_SYS_FILE_H)
+# include <sys/file.h>
+#endif
+
+#ifdef NO_OUTPUT_SYNC
+# define RECORD_SYNC_MUTEX(m) \
+    O (error, NILF,                                                    \
+       _("-O[TYPE] (--output-sync[=TYPE]) is not configured for this build."));
+#else
 int output_tmpfd (void);
 /* Dump any child output content to stdout, and reset it.  */
 void output_dump (struct output *out);
-#endif
+
+# ifdef WINDOWS32
+/* For emulations in w32/compat/posixfcn.c.  */
+#  define F_GETFD 1
+#  define F_SETLKW 2
+/* Implementation note: None of the values of l_type below can be zero
+   -- they are compared with a static instance of the struct, so zero
+   means unknown/invalid, see w32/compat/posixfcn.c. */
+#  define F_WRLCK 1
+#  define F_UNLCK 2
+
+struct flock
+  {
+    short l_type;
+    short l_whence;
+    off_t l_start;
+    off_t l_len;
+    pid_t l_pid;
+  };
+
+/* This type is actually a HANDLE, but we want to avoid including
+   windows.h as much as possible.  */
+typedef intptr_t sync_handle_t;
+
+/* Public functions emulated/provided in posixfcn.c.  */
+int fcntl (intptr_t fd, int cmd, ...);
+intptr_t create_mutex (void);
+int same_stream (FILE *f1, FILE *f2);
+
+#  define RECORD_SYNC_MUTEX(m) record_sync_mutex(m)
+void record_sync_mutex (const char *str);
+void prepare_mutex_handle_string (intptr_t hdl);
+# else  /* !WINDOWS32 */
+
+typedef int sync_handle_t;      /* file descriptor */
+
+#  define RECORD_SYNC_MUTEX(m) (void)(m)
+
+# endif
+#endif  /* !NO_OUTPUT_SYNC */

diff --git a/po/POTFILES.in b/po/POTFILES.in
index 061ff2b93f65da38810e0f9f4c6537c2fa7086be..78b8c851b16a90ec1052f32a992857b03e427cab 100644 (file)
--- a/po/POTFILES.in
+++ b/po/POTFILES.in
@@ -27,11 +27,11 @@ guile.c
 hash.c
 implicit.c
 job.c
-job.h
 load.c
 main.c
 misc.c
 output.c
+output.h
 posixos.c
 read.c
 remake.c

diff --git a/posixos.c b/posixos.c
index 4a787e4d01111265ade37a981e3d8ab907af49b3..e642d7f3bd99190fe49b2bd6bc323faeb290f0f2 100644 (file)
--- a/posixos.c
+++ b/posixos.c
@@ -20,7 +20,10 @@ this program.  If not, see <http://www.gnu.org/licenses/>.  */
 
 #ifdef HAVE_FCNTL_H
 # include <fcntl.h>
+#elif defined(HAVE_SYS_FILE_H)
+# include <sys/file.h>
 #endif
+
 #if defined(HAVE_PSELECT) && defined(HAVE_SYS_SELECT_H)
 # include <sys/select.h>
 #endif
@@ -53,7 +56,7 @@ make_job_rfd (void)
 #else
   EINTRLOOP (job_rfd, dup (job_fds[0]));
   if (job_rfd >= 0)
-    CLOSE_ON_EXEC (job_rfd);
+    fd_noinherit (job_rfd);
 
   return job_rfd;
 #endif
@@ -68,6 +71,11 @@ jobserver_setup (int slots)
   if (r < 0)
     pfatal_with_name (_("creating jobs pipe"));
 
+  /* By default we don't send the job pipe FDs to our children.
+     See jobserver_pre_child() and jobserver_post_child().  */
+  fd_noinherit (job_fds[0]);
+  fd_noinherit (job_fds[1]);
+
   if (make_job_rfd () < 0)
     pfatal_with_name (_("duping jobs pipe"));
 
@@ -180,33 +188,22 @@ jobserver_acquire_all (void)
 void
 jobserver_pre_child (int recursive)
 {
-  /* If it's not a recursive make, avoid polutting the jobserver pipes.  */
-  if (!recursive && job_fds[0] >= 0)
+  if (recursive && job_fds[0] >= 0)
     {
-      CLOSE_ON_EXEC (job_fds[0]);
-      CLOSE_ON_EXEC (job_fds[1]);
+      fd_inherit (job_fds[0]);
+      fd_inherit (job_fds[1]);
     }
 }
 
+/* Reconfigure the jobserver after starting a child process.  */
 void
 jobserver_post_child (int recursive)
 {
-#if defined(F_GETFD) && defined(F_SETFD)
-  if (!recursive && job_fds[0] >= 0)
+  if (recursive && job_fds[0] >= 0)
     {
-      unsigned int i;
-      for (i = 0; i < 2; ++i)
-        {
-          int flags;
-          EINTRLOOP (flags, fcntl (job_fds[i], F_GETFD));
-          if (flags >= 0)
-            {
-              int r;
-              EINTRLOOP (r, fcntl (job_fds[i], F_SETFD, flags & ~FD_CLOEXEC));
-            }
-        }
+      fd_noinherit (job_fds[0]);
+      fd_noinherit (job_fds[1]);
     }
-#endif
 }
 
 void
@@ -396,7 +393,7 @@ jobserver_acquire (int timeout)
   return 0;
 }
 
-#endif
+#endif /* HAVE_PSELECT */
 
 #endif /* MAKE_JOBSERVER */
 
@@ -423,9 +420,48 @@ get_bad_stdin (void)
           /* Set the descriptor to close on exec, so it does not litter any
              child's descriptor table.  When it is dup2'd onto descriptor 0,
              that descriptor will not close on exec.  */
-          CLOSE_ON_EXEC (bad_stdin);
+          fd_noinherit (bad_stdin);
         }
     }
 
   return bad_stdin;
 }
+
+/* Set file descriptors to be inherited / not inherited by subprocesses.  */
+
+#if !defined(F_SETFD) || !defined(F_GETFD)
+void fd_inherit (int fd) {}
+void fd_noinherit (int fd) {}
+
+#else
+
+# ifndef FD_CLOEXEC
+#  define FD_CLOEXEC 1
+# endif
+
+void
+fd_inherit (int fd)
+{
+  int flags;
+  EINTRLOOP (flags, fcntl (fd, F_GETFD));
+  if (flags >= 0)
+    {
+      int r;
+      flags &= ~FD_CLOEXEC;
+      EINTRLOOP (r, fcntl (fd, F_SETFD, flags));
+    }
+}
+
+void
+fd_noinherit (int fd)
+{
+    int flags;
+    EINTRLOOP(flags, fcntl(fd, F_GETFD));
+    if (flags >= 0)
+      {
+        int r;
+        flags |= FD_CLOEXEC;
+        EINTRLOOP(r, fcntl(fd, F_SETFD, flags));
+      }
+}
+#endif

diff --git a/read.c b/read.c
index 5d84a13adeb7e2cba496ffbe3e305b0abc8a0923..3e3998482eedf197ece4806c8a934310bb78058e 100644 (file)
--- a/read.c
+++ b/read.c
@@ -21,6 +21,7 @@ this program.  If not, see <http://www.gnu.org/licenses/>.  */
 #include "filedef.h"
 #include "dep.h"
 #include "job.h"
+#include "os.h"
 #include "commands.h"
 #include "variable.h"
 #include "rule.h"
@@ -415,11 +416,8 @@ eval_makefile (const char *filename, int flags)
   /* Success; clear errno.  */
   deps->error = 0;
 
-  /* Set close-on-exec to avoid leaking the makefile to children, such as
-     $(shell ...).  */
-#ifdef HAVE_FILENO
-  CLOSE_ON_EXEC (fileno (ebuf.fp));
-#endif
+  /* Avoid leaking the makefile to children.  */
+  fd_noinherit (fileno (ebuf.fp));
 
   /* Add this makefile to the list. */
   do_variable_definition (&ebuf.floc, "MAKEFILE_LIST", filename, o_file,

diff --git a/w32/include/sub_proc.h b/w32/include/sub_proc.h
index 4b7f10fef4de2d3531f6ba5ccc835790be697a1f..f2be0bedeedfe88bd112c00df068c878ce2185ec 100644 (file)
--- a/w32/include/sub_proc.h
+++ b/w32/include/sub_proc.h
@@ -60,6 +60,5 @@ EXTERN_DECL(char * process_errbuf, (HANDLE proc));
 EXTERN_DECL(int process_outcnt, (HANDLE proc));
 EXTERN_DECL(int process_errcnt, (HANDLE proc));
 EXTERN_DECL(void process_pipes, (HANDLE proc, int pipes[3]));
-EXTERN_DECL(void process_noinherit, (int fildes));
 
 #endif

diff --git a/w32/subproc/sub_proc.c b/w32/subproc/sub_proc.c
index 69b3ae1351c2994017bb15e77eb3cf5a9784bcbf..2a6eaaecaf7d36129794a95531a32059b2284ee3 100644 (file)
--- a/w32/subproc/sub_proc.c
+++ b/w32/subproc/sub_proc.c
@@ -339,15 +339,6 @@ process_exit_code(HANDLE proc)
         return (((sub_process *)proc)->exit_code);
 }
 
-void
-process_noinherit(int fd)
-{
-  HANDLE fh = (HANDLE)_get_osfhandle(fd);
-
-  if (fh && fh != INVALID_HANDLE_VALUE)
-        SetHandleInformation(fh, HANDLE_FLAG_INHERIT, 0);
-}
-
 /*
 2006-02:
 All the following functions are currently unused.

diff --git a/w32/w32os.c b/w32/w32os.c
index b1485fe24300f90f74775210b5e2dafec104db8e..4f35051bc471dfe7ce309cbe678eb25a59389f97 100644 (file)
--- a/w32/w32os.c
+++ b/w32/w32os.c
@@ -198,3 +198,21 @@ jobserver_acquire (int timeout)
     /* WAIT_OBJECT_0 indicates that the semaphore was signalled.  */
     return dwEvent == WAIT_OBJECT_0;
 }
+
+void
+fd_inherit(int fd)
+{
+  HANDLE fh = (HANDLE)_get_osfhandle(fd);
+
+  if (fh && fh != INVALID_HANDLE_VALUE)
+        SetHandleInformation(fh, HANDLE_FLAG_INHERIT, 1);
+}
+
+void
+fd_noinherit(int fd)
+{
+  HANDLE fh = (HANDLE)_get_osfhandle(fd);
+
+  if (fh && fh != INVALID_HANDLE_VALUE)
+        SetHandleInformation(fh, HANDLE_FLAG_INHERIT, 0);
+}