bug-gnulib
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: add pipe2-safer


From: Bruno Haible
Subject: Re: add pipe2-safer
Date: Fri, 11 Dec 2009 17:39:59 +0100
User-agent: KMail/1.9.9

Eric Blake wrote:
> It looks okay, except for:
> 
> > +          fd[i] = fd_safer_flag (fd[i], flags);
> 
> unistd-safer.h doesn't declare fd_safer_flag unless the cloexec module was
> also in use.  Therefore,
> 
> >  Depends-on:
> > -cloexec
> 
> ...I think this portion of the change is incorrect.

OK, committed without changing the 'cloexec' dependency.

It's weird that the dup_safer_flag and fd_safer_flag get defined if the
module 'unistd-safer' AND the module 'cloexec' are included. If a package
needs pipe_safer and set_cloexec_flag, why should it compile dup_safer_flag
and fd_safer_flag?

Everything becomes simpler when there is a clear relation
  "need function X => must request module Y"
for every C function X.

Also, the functions mkostemp_safer (compiled when module 'stdlib-safer'
AND module 'mkostemp' are requested) and mkostemps_safer (compiled when
module 'stdlib-safer' AND module 'mkostemps' are requested) also rely on
fd_safer_flag, but neither of 'stdlib-safer', 'mkostemp', 'mkostemps'
depends on 'cloexec'. It's very hard to get dependencies right when you
have code that is conditional on module1 AND module2.

Here is a proposed patch to fix this:


2009-12-11  Bruno Haible  <address@hidden>

        New module 'fd-safer-flag'.
        * lib/dup-safer-flag.c: New file, extracted from lib/dup-safer.c.
        * lib/dup-safer.c (dup_safer_flag): Remove function.
        * lib/fd-safer-flag.c: New file, extracted from lib/fd-safer.c.
        * lib/fd-safer.c (fd_safer_flag): Remove function.
        * lib/unistd-safer.h (dup_safer_flag, fd_safer_flag): Update condition.
        * modules/cloexec (configure.ac): Drop indicator macro.
        * modules/fd-safer-flag: New file.
        * modules/pipe2-safer (Depends-on): Add fd-safer-flag. Remove cloexec.
        * modules/stdlib-safer (Depends-on): Add fd-safer-flag.
        * modules/unistd-safer-tests (Depends-on): Add fd-safer-flag.

Changing permissions from . to 100644
--- lib/dup-safer-flag.c.orig   2009-04-14 12:31:40.000000000 +0200
+++ lib/dup-safer-flag.c        2009-12-11 16:56:48.000000000 +0100
@@ -0,0 +1,54 @@
+/* Duplicate a file descriptor result, avoiding clobbering
+   STD{IN,OUT,ERR}_FILENO, with specific flags.
+
+   Copyright (C) 2001, 2004-2006, 2009 Free Software Foundation, Inc.
+
+   This program is free software: you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
+
+/* Written by Paul Eggert and Eric Blake.  */
+
+#include <config.h>
+
+/* Specification.  */
+#include "unistd-safer.h"
+
+#include <fcntl.h>
+#include <unistd.h>
+
+#include "cloexec.h"
+
+#ifndef O_CLOEXEC
+# define O_CLOEXEC 0
+#endif
+
+/* Like dup, but do not return STDIN_FILENO, STDOUT_FILENO, or
+   STDERR_FILENO.  If FLAG contains O_CLOEXEC, behave like
+   fcntl(F_DUPFD_CLOEXEC) rather than fcntl(F_DUPFD).  */
+
+int
+dup_safer_flag (int fd, int flag)
+{
+  if (flag & O_CLOEXEC)
+    {
+#if defined F_DUPFD_CLOEXEC && !REPLACE_FCHDIR
+      return fcntl (fd, F_DUPFD_CLOEXEC, STDERR_FILENO + 1);
+#else
+      /* fd_safer_flag calls us back, but eventually the recursion
+         unwinds and does the right thing.  */
+      fd = dup_cloexec (fd);
+      return fd_safer_flag (fd, flag);
+#endif
+    }
+  return dup_safer (fd);
+}
--- lib/dup-safer.c.orig        2009-12-11 17:37:55.000000000 +0100
+++ lib/dup-safer.c     2009-12-11 16:56:52.000000000 +0100
@@ -39,34 +39,3 @@
   return fd_safer (dup (fd));
 #endif
 }
-
-#if GNULIB_CLOEXEC
-
-# include "cloexec.h"
-
-# ifndef O_CLOEXEC
-#  define O_CLOEXEC 0
-# endif
-
-/* Like dup, but do not return STDIN_FILENO, STDOUT_FILENO, or
-   STDERR_FILENO.  If FLAG contains O_CLOEXEC, behave like
-   fcntl(F_DUPFD_CLOEXEC) rather than fcntl(F_DUPFD).  */
-
-int
-dup_safer_flag (int fd, int flag)
-{
-  if (flag & O_CLOEXEC)
-    {
-# if defined F_DUPFD_CLOEXEC && !REPLACE_FCHDIR
-      return fcntl (fd, F_DUPFD_CLOEXEC, STDERR_FILENO + 1);
-# else
-      /* fd_safer_flag calls us back, but eventually the recursion
-         unwinds and does the right thing.  */
-      fd = dup_cloexec (fd);
-      return fd_safer_flag (fd, flag);
-# endif
-    }
-  return dup_safer (fd);
-}
-
-#endif
Changing permissions from . to 100644
--- lib/fd-safer-flag.c.orig    2009-04-14 12:31:40.000000000 +0200
+++ lib/fd-safer-flag.c 2009-12-11 16:58:16.000000000 +0100
@@ -0,0 +1,52 @@
+/* Adjust a file descriptor result so that it avoids clobbering
+   STD{IN,OUT,ERR}_FILENO, with specific flags.
+
+   Copyright (C) 2005-2006, 2009 Free Software Foundation, Inc.
+
+   This program is free software: you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
+
+/* Written by Paul Eggert and Eric Blake.  */
+
+#include <config.h>
+
+/* Specification.  */
+#include "unistd-safer.h"
+
+#include <errno.h>
+#include <unistd.h>
+
+/* Return FD, unless FD would be a copy of standard input, output, or
+   error; in that case, return a duplicate of FD, closing FD.  If FLAG
+   contains O_CLOEXEC, the returned FD will have close-on-exec
+   semantics.  On failure to duplicate, close FD, set errno, and
+   return -1.  Preserve errno if FD is negative, so that the caller
+   can always inspect errno when the returned value is negative.
+
+   This function is usefully wrapped around functions that return file
+   descriptors, e.g., fd_safer_flag (open ("file", O_RDONLY | flag), flag).  */
+
+int
+fd_safer_flag (int fd, int flag)
+{
+  if (STDIN_FILENO <= fd && fd <= STDERR_FILENO)
+    {
+      int f = dup_safer_flag (fd, flag);
+      int e = errno;
+      close (fd);
+      errno = e;
+      fd = f;
+    }
+
+  return fd;
+}
--- lib/fd-safer.c.orig 2009-12-11 17:37:55.000000000 +0100
+++ lib/fd-safer.c      2009-12-11 16:58:33.000000000 +0100
@@ -22,7 +22,6 @@
 #include "unistd-safer.h"
 
 #include <errno.h>
-
 #include <unistd.h>
 
 /* Return FD, unless FD would be a copy of standard input, output, or
@@ -48,32 +47,3 @@
 
   return fd;
 }
-
-#if GNULIB_CLOEXEC
-
-/* Return FD, unless FD would be a copy of standard input, output, or
-   error; in that case, return a duplicate of FD, closing FD.  If FLAG
-   contains O_CLOEXEC, the returned FD will have close-on-exec
-   semantics.  On failure to duplicate, close FD, set errno, and
-   return -1.  Preserve errno if FD is negative, so that the caller
-   can always inspect errno when the returned value is negative.
-
-   This function is usefully wrapped around functions that return file
-   descriptors, e.g., fd_safer_flag (open ("file", O_RDONLY | flag), flag).  */
-
-int
-fd_safer_flag (int fd, int flag)
-{
-  if (STDIN_FILENO <= fd && fd <= STDERR_FILENO)
-    {
-      int f = dup_safer_flag (fd, flag);
-      int e = errno;
-      close (fd);
-      errno = e;
-      fd = f;
-    }
-
-  return fd;
-}
-
-#endif /* GNULIB_CLOEXEC */
--- lib/unistd-safer.h.orig     2009-12-11 17:37:55.000000000 +0100
+++ lib/unistd-safer.h  2009-12-11 16:59:02.000000000 +0100
@@ -21,7 +21,7 @@
 int fd_safer (int);
 int pipe_safer (int[2]);
 
-#if GNULIB_CLOEXEC
+#if GNULIB_FD_SAFER_FLAG
 int dup_safer_flag (int, int);
 int fd_safer_flag (int, int);
 #endif
--- modules/cloexec.orig        2009-12-11 17:37:55.000000000 +0100
+++ modules/cloexec     2009-12-11 17:01:00.000000000 +0100
@@ -12,7 +12,6 @@
 
 configure.ac:
 gl_CLOEXEC
-gl_MODULE_INDICATOR([cloexec])
 
 Makefile.am:
 
Changing permissions from . to 100644
--- modules/fd-safer-flag.orig  2009-04-14 12:31:40.000000000 +0200
+++ modules/fd-safer-flag       2009-12-11 17:00:14.000000000 +0100
@@ -0,0 +1,26 @@
+Description:
+fd_safer_flag() function: adjust a file descriptor result so that it avoids
+clobbering STD{IN,OUT,ERR}_FILENO, with specific flags.
+
+Files:
+lib/fd-safer-flag.c
+lib/dup-safer-flag.c
+
+Depends-on:
+unistd-safer
+cloexec
+
+configure.ac:
+gl_MODULE_INDICATOR([fd-safer-flag])
+
+Makefile.am:
+lib_SOURCES += fd-safer-flag.c dup-safer-flag.c
+
+Include:
+"unistd-safer.h"
+
+License:
+GPL
+
+Maintainer:
+Eric Blake
--- modules/pipe2-safer.orig    2009-12-11 17:37:55.000000000 +0100
+++ modules/pipe2-safer 2009-12-11 16:59:28.000000000 +0100
@@ -6,7 +6,7 @@
 lib/pipe2-safer.c
 
 Depends-on:
-cloexec
+fd-safer-flag
 pipe2
 unistd-safer
 
--- modules/stdlib-safer.orig   2009-12-11 17:37:55.000000000 +0100
+++ modules/stdlib-safer        2009-12-11 17:04:42.000000000 +0100
@@ -8,6 +8,7 @@
 m4/stdlib-safer.m4
 
 Depends-on:
+fd-safer-flag
 mkstemp
 stdlib
 unistd-safer
--- modules/unistd-safer-tests.orig     2009-12-11 17:37:55.000000000 +0100
+++ modules/unistd-safer-tests  2009-12-11 17:31:29.000000000 +0100
@@ -4,6 +4,7 @@
 Depends-on:
 binary-io
 cloexec
+fd-safer-flag
 stdbool
 
 configure.ac:





reply via email to

[Prev in Thread] Current Thread [Next in Thread]