This is the mail archive of the
libc-alpha@sourceware.org
mailing list for the glibc project.
Re: [PATCH] Assume that dup3 is available
- From: Adhemerval Zanella <adhemerval dot zanella at linaro dot org>
- To: libc-alpha at sourceware dot org
- Date: Mon, 17 Apr 2017 15:16:27 -0300
- Subject: Re: [PATCH] Assume that dup3 is available
- Authentication-results: sourceware.org; auth=none
- References: <20170413192548.BF5A040EBCF62@oldenburg.str.redhat.com>
LGTM with with one remark below.
On 13/04/2017 16:25, Florian Weimer wrote:
> 2017-04-13 Florian Weimer <fweimer@redhat.com>
>
> * include/unistd.h (__have_dup3): Remove declaration.
> * libio/freopen.c (freopen): Assume that O_CLOEXEC is defined and
> dup3 is available.
> * libio/freopen64.c (freopen64): Likewise.
> * socket/Makefile (aux): Remove have_sock_cloexec.
> * socket/have_sock_cloexec.c: Remove file.
> * sysdeps/mach/hurd/kernel-features.h (__ASSUME_DUP3): Remove
> definition.
> * sysdeps/unix/sysv/linux/kernel-features.h (__ASSUME_DUP3):
> Likewise.
> * sysdeps/nacl/kernel-features.h: Update comment.
>
> diff --git a/include/unistd.h b/include/unistd.h
> index e15fa0e..16a8815 100644
> --- a/include/unistd.h
> +++ b/include/unistd.h
> @@ -171,8 +171,6 @@ extern int __libc_pause (void);
> /* Not cancelable variant. */
> extern int __pause_nocancel (void) attribute_hidden;
>
> -extern int __have_dup3 attribute_hidden;
> -
> extern int __getlogin_r_loginuid (char *name, size_t namesize)
> attribute_hidden;
>
> diff --git a/libio/freopen.c b/libio/freopen.c
> index 03e3ae7..ad1c848 100644
> --- a/libio/freopen.c
> +++ b/libio/freopen.c
> @@ -78,32 +78,9 @@ freopen (const char *filename, const char *mode, FILE *fp)
>
> if (fd != -1)
> {
> -#ifdef O_CLOEXEC
> -# ifndef __ASSUME_DUP3
> - int newfd;
> - if (__have_dup3 < 0)
> - newfd = -1;
> - else
> - newfd =
> -# endif
> - __dup3 (_IO_fileno (result), fd,
> - (result->_flags2 & _IO_FLAGS2_CLOEXEC) != 0
> - ? O_CLOEXEC : 0);
> -#else
> -# define newfd 1
> -#endif
> -
> -#ifndef __ASSUME_DUP3
> - if (newfd < 0)
> - {
> - if (errno == ENOSYS)
> - __have_dup3 = -1;
> -
> - __dup2 (_IO_fileno (result), fd);
> - if ((result->_flags2 & _IO_FLAGS2_CLOEXEC) != 0)
> - __fcntl (fd, F_SETFD, FD_CLOEXEC);
> - }
> -#endif
> + __dup3 (_IO_fileno (result), fd,
> + (result->_flags2 & _IO_FLAGS2_CLOEXEC) != 0
> + ? O_CLOEXEC : 0);
Shouldn't we check the result of __dup3 and act accordingly in this situation
(basically EINTR and EMFILE)?
> __close (_IO_fileno (result));
> _IO_fileno (result) = fd;
> }
> diff --git a/libio/freopen64.c b/libio/freopen64.c
> index 1f0d8ab..adf749a 100644
> --- a/libio/freopen64.c
> +++ b/libio/freopen64.c
> @@ -61,32 +61,9 @@ freopen64 (const char *filename, const char *mode, FILE *fp)
>
> if (fd != -1)
> {
> -#ifdef O_CLOEXEC
> -# ifndef __ASSUME_DUP3
> - int newfd;
> - if (__have_dup3 < 0)
> - newfd = -1;
> - else
> - newfd =
> -# endif
> - __dup3 (_IO_fileno (result), fd,
> - (result->_flags2 & _IO_FLAGS2_CLOEXEC) != 0
> - ? O_CLOEXEC : 0);
> -#else
> -# define newfd 1
> -#endif
> -
> -#ifndef __ASSUME_DUP3
> - if (newfd < 0)
> - {
> - if (errno == ENOSYS)
> - __have_dup3 = -1;
> -
> - __dup2 (_IO_fileno (result), fd);
> - if ((result->_flags2 & _IO_FLAGS2_CLOEXEC) != 0)
> - __fcntl (fd, F_SETFD, FD_CLOEXEC);
> - }
> -#endif
> + __dup3 (_IO_fileno (result), fd,
> + (result->_flags2 & _IO_FLAGS2_CLOEXEC) != 0
> + ? O_CLOEXEC : 0);
Same as before.
> __close (_IO_fileno (result));
> _IO_fileno (result) = fd;
> }
> diff --git a/socket/Makefile b/socket/Makefile
> index c299d34..25d4f68 100644
> --- a/socket/Makefile
> +++ b/socket/Makefile
> @@ -31,6 +31,6 @@ routines := accept bind connect getpeername getsockname getsockopt \
> setsockopt shutdown socket socketpair isfdtype opensock \
> sockatmark accept4 recvmmsg sendmmsg
>
> -aux := have_sock_cloexec sa_len
> +aux := sa_len
>
> include ../Rules
> diff --git a/socket/have_sock_cloexec.c b/socket/have_sock_cloexec.c
> deleted file mode 100644
> index 579577d..0000000
> --- a/socket/have_sock_cloexec.c
> +++ /dev/null
> @@ -1,24 +0,0 @@
> -/* Copyright (C) 2008-2017 Free Software Foundation, Inc.
> - This file is part of the GNU C Library.
> -
> - The GNU C Library is free software; you can redistribute it and/or
> - modify it under the terms of the GNU Lesser General Public
> - License as published by the Free Software Foundation; either
> - version 2.1 of the License, or (at your option) any later version.
> -
> - The GNU C Library 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
> - Lesser General Public License for more details.
> -
> - You should have received a copy of the GNU Lesser General Public
> - License along with the GNU C Library; if not, see
> - <http://www.gnu.org/licenses/>. */
> -
> -#include <fcntl.h>
> -#include <sys/socket.h>
> -#include <kernel-features.h>
> -
> -#if defined O_CLOEXEC && !defined __ASSUME_DUP3
> -int __have_dup3;
> -#endif
> diff --git a/sysdeps/mach/hurd/kernel-features.h b/sysdeps/mach/hurd/kernel-features.h
> index 687c7f0..60c5bca 100644
> --- a/sysdeps/mach/hurd/kernel-features.h
> +++ b/sysdeps/mach/hurd/kernel-features.h
> @@ -21,5 +21,4 @@
> But those referring to POSIX-level features like O_* flags can be. */
>
> #define __ASSUME_O_CLOEXEC 1
> -#define __ASSUME_DUP3 1
> #define __ASSUME_ACCEPT4 1
> diff --git a/sysdeps/nacl/kernel-features.h b/sysdeps/nacl/kernel-features.h
> index 38a47f1..4a5808b 100644
> --- a/sysdeps/nacl/kernel-features.h
> +++ b/sysdeps/nacl/kernel-features.h
> @@ -23,6 +23,5 @@
> #define __ASSUME_O_CLOEXEC 1
>
> /*
> -#define __ASSUME_DUP3 1
> #define __ASSUME_ACCEPT4 1
> */
> diff --git a/sysdeps/unix/sysv/linux/kernel-features.h b/sysdeps/unix/sysv/linux/kernel-features.h
> index 233e302..e0eb4e2 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_DUP3 1
>
> /* Support for accept4 functionality was added in 2.6.28, but for some
> architectures using a separate syscall rather than socketcall that
>