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 <br...@clisp.org>
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:
