[PATCH] Assume that pipe2 is always available
Adhemerval Zanella
adhemerval.zanella@linaro.org
Thu Apr 13 17:39:00 GMT 2017
On 13/04/2017 12:37, Florian Weimer wrote:
> The Debian patches (which are already required to build glibc before
> this commit) contain an implementation of pipe2.
I did not follow, which 'Debian patches' are you referring here?
>
> 2017-04-13 Florian Weimer <fweimer@redhat.com>
>
> * include/unistd.h (__have_pipe2): Remove declaration.
> * socket/have_sock_cloexec.c (__have_pipe2): Remove definition.
> * libio/iopopen.c (_IO_new_proc_open): Assume that pipe2 is
> available.
> * posix/wordexp.c (exec_comm_child, exec_comm): Likewise.
> * sysdeps/unix/sysv/linux/kernel-features.h (__ASSUME_PIPE2):
> Remove definition.
>
> diff --git a/include/unistd.h b/include/unistd.h
> index 16d68a1..e15fa0e 100644
> --- a/include/unistd.h
> +++ b/include/unistd.h
> @@ -171,7 +171,6 @@ extern int __libc_pause (void);
> /* Not cancelable variant. */
> extern int __pause_nocancel (void) attribute_hidden;
>
> -extern int __have_pipe2 attribute_hidden;
> extern int __have_dup3 attribute_hidden;
>
> extern int __getlogin_r_loginuid (char *name, size_t namesize)
> diff --git a/libio/iopopen.c b/libio/iopopen.c
> index 08e29b4..5887bd1 100644
> --- a/libio/iopopen.c
> +++ b/libio/iopopen.c
> @@ -141,28 +141,12 @@ _IO_new_proc_open (_IO_FILE *fp, const char *command, const char *mode)
> return NULL;
>
> #ifdef O_CLOEXEC
> -# ifndef __ASSUME_PIPE2
> - if (__have_pipe2 >= 0)
> -# endif
> {
> int r = __pipe2 (pipe_fds, O_CLOEXEC);
> -# ifndef __ASSUME_PIPE2
> - if (__have_pipe2 == 0)
> - __have_pipe2 = r != -1 || errno != ENOSYS ? 1 : -1;
> -
> - if (__have_pipe2 > 0)
> -# endif
> if (r < 0)
> return NULL;
> }
> #endif
> -#ifndef __ASSUME_PIPE2
> -# ifdef O_CLOEXEC
> - if (__have_pipe2 < 0)
> -# endif
> - if (__pipe (pipe_fds) < 0)
> - return NULL;
> -#endif
Why not remove the bracket and just use something like:
#ifdef O_CLOCEXEC
if (__pipe2 (pipe_fds, O_CLOCEXEC) != 0)
return NULL
#endif
>
> if (do_read)
> {
> @@ -183,27 +167,13 @@ _IO_new_proc_open (_IO_FILE *fp, const char *command, const char *mode)
> int child_std_end = do_read ? 1 : 0;
> struct _IO_proc_file *p;
>
> -#ifndef __ASSUME_PIPE2
> - /* If we have pipe2 the descriptor is marked for close-on-exec. */
> - _IO_close (parent_end);
> -#endif
> if (child_end != child_std_end)
> - {
> - _IO_dup2 (child_end, child_std_end);
> -#ifndef __ASSUME_PIPE2
> - _IO_close (child_end);
> -#endif
> - }
> + _IO_dup2 (child_end, child_std_end);
> #ifdef O_CLOEXEC
> else
> - {
> - /* The descriptor is already the one we will use. But it must
> - not be marked close-on-exec. Undo the effects. */
> -# ifndef __ASSUME_PIPE2
> - if (__have_pipe2 > 0)
> -# endif
> - __fcntl (child_end, F_SETFD, 0);
> - }
> + /* The descriptor is already the one we will use. But it must
> + not be marked close-on-exec. Undo the effects. */
> + __fcntl (child_end, F_SETFD, 0);
> #endif
> /* POSIX.2: "popen() shall ensure that any streams from previous
> popen() calls that remain open in the parent process are closed
> @@ -229,26 +199,12 @@ _IO_new_proc_open (_IO_FILE *fp, const char *command, const char *mode)
> return NULL;
> }
>
> - if (do_cloexec)
> - {
> -#ifndef __ASSUME_PIPE2
> -# ifdef O_CLOEXEC
> - if (__have_pipe2 < 0)
> -# endif
> - __fcntl (parent_end, F_SETFD, FD_CLOEXEC);
> -#endif
> - }
> - else
> - {
> + if (!do_cloexec)
> #ifdef O_CLOEXEC
> - /* Undo the effects of the pipe2 call which set the
> - close-on-exec flag. */
> -# ifndef __ASSUME_PIPE2
> - if (__have_pipe2 > 0)
> -# endif
> - __fcntl (parent_end, F_SETFD, 0);
> + /* Undo the effects of the pipe2 call which set the
> + close-on-exec flag. */
> + __fcntl (parent_end, F_SETFD, 0);
> #endif
> - }
>
> _IO_fileno (fp) = parent_end;
>
> diff --git a/posix/wordexp.c b/posix/wordexp.c
> index ba3f3ed..639d73e 100644
> --- a/posix/wordexp.c
> +++ b/posix/wordexp.c
> @@ -836,10 +836,7 @@ exec_comm_child (char *comm, int *fildes, int showerr, int noexec)
> {
> #ifdef O_CLOEXEC
> /* Reset the close-on-exec flag (if necessary). */
> -# ifndef __ASSUME_PIPE2
> - if (__have_pipe2 > 0)
> -# endif
> - __fcntl (fildes[1], F_SETFD, 0);
> + __fcntl (fildes[1], F_SETFD, 0);
> #endif
> }
>
> @@ -906,29 +903,12 @@ exec_comm (char *comm, char **word, size_t *word_length, size_t *max_length,
> return 0;
>
> #ifdef O_CLOEXEC
> -# ifndef __ASSUME_PIPE2
> - if (__have_pipe2 >= 0)
> -# endif
> - {
> - int r = __pipe2 (fildes, O_CLOEXEC);
> -# ifndef __ASSUME_PIPE2
> - if (__have_pipe2 == 0)
> - __have_pipe2 = r != -1 || errno != ENOSYS ? 1 : -1;
> -
> - if (__have_pipe2 > 0)
> -# endif
> - if (r < 0)
> - /* Bad */
> - return WRDE_NOSPACE;
> - }
> -#endif
> -#ifndef __ASSUME_PIPE2
> -# ifdef O_CLOEXEC
> - if (__have_pipe2 < 0)
> -# endif
> - if (__pipe (fildes) < 0)
> + {
> + int r = __pipe2 (fildes, O_CLOEXEC);
> + if (r < 0)
> /* Bad */
> return WRDE_NOSPACE;
> + }
> #endif
Same as before, I think it is simpler to just remove the brackets.
>
> again:
> diff --git a/socket/have_sock_cloexec.c b/socket/have_sock_cloexec.c
> index 70c730e..579577d 100644
> --- a/socket/have_sock_cloexec.c
> +++ b/socket/have_sock_cloexec.c
> @@ -19,10 +19,6 @@
> #include <sys/socket.h>
> #include <kernel-features.h>
>
> -#if defined O_CLOEXEC && !defined __ASSUME_PIPE2
> -int __have_pipe2;
> -#endif
> -
> #if defined O_CLOEXEC && !defined __ASSUME_DUP3
> int __have_dup3;
> #endif
> diff --git a/sysdeps/unix/sysv/linux/kernel-features.h b/sysdeps/unix/sysv/linux/kernel-features.h
> index e6a2720..233e302 100644
> --- a/sysdeps/unix/sysv/linux/kernel-features.h
> +++ b/sysdeps/unix/sysv/linux/kernel-features.h
> @@ -74,7 +74,6 @@
> /* Support for various CLOEXEC and NONBLOCK flags was added in
> 2.6.27. */
> #define __ASSUME_IN_NONBLOCK 1
> -#define __ASSUME_PIPE2 1
> #define __ASSUME_DUP3 1
>
> /* Support for accept4 functionality was added in 2.6.28, but for some
>
More information about the Libc-alpha
mailing list