Message ID | 2e3b2c9a-af57-423e-29e8-8f46eb5478b6@redhat.com |
---|---|
State | New |
Headers | show |
On 18/04/2017 16:37, Florian Weimer wrote: > On 04/17/2017 05:01 PM, Adhemerval Zanella wrote: >>> + { >>> + socklen_t sa_len = sizeof (server_address); >>> + xgetsockname (server_socket, (struct sockaddr *) &server_address, >>> + &sa_len); >>> + TEST_VERIFY (sa_len == sizeof (server_address)); >>> + } > >> Why the brackets? I usually seems this unnecessary. > > I prefer to restrict the scope of local variables if possible. Ok. > >> Now that we are assuming accept4 support regardless, I think >> we can simplify it even more and just have either >> __ASSUME_ACCEPT4_SYSCALL or __ASSUME_SOCKETCALL >> (which would imply __ASSUME_ACCEPT4_SYSCALL_WITH_SOCKETCALL). > > We currently need all those because some override headers include sysdeps/unix/sysv/linux/kernel-features.h before their own definitions, others after. Rectifying that is a bit involved, and I don't want to do it with this change. > All right, I think we can cleanup this in another patch (I will prob work on it when you patch lands). > I believe I have addressed your other suggestions with the attached patch. Yes, I think it is all covered, thanks. > > Thanks, > Florian > > accept4.patch > > > Assume that accept4 is always available and works > > Simplify the Linux accept4 implementation based on the assumption > that it is available in some way. __ASSUME_ACCEPT4_SOCKETCALL was > previously unused, so remove it. > > For ia64, the accept4 system call (and socket call) were backported > in kernel version 3.2.18. Reflect this in the installation > instructions. > > 2017-04-18 Florian Weimer <fweimer@redhat.com> > > * nscd/connections.c (have_accept4): Removed definition. > (nscd_run_worker, main_loop_poll, main_loop_epolll): Assume that > accept4 works. > * manual/install.texi (Linux): Require at least kernel 3.2.18 for > ia64 because that was the first version with accept4 support. > * support/Makefile (libsupport-routines): Add xaccept4. > * support/xsocket.h (xaccept4): Declare. > * support/xaccept4.c: New file. > * socket/tst-accept4.c: New file. > * socket/Makefile (tests): Add tst-accept4. > * sysdeps/mach/hurd/kernel-features.h (__ASSUME_ACCEPT4): Remove > definition. > * sysdeps/nacl/kernel-features.h (__ASSUME_ACCEPT4): Remove > comment. > * sysdeps/unix/sysv/linux/accept4.c: Assume that an accept4 > implementation is available. > (accept4): Use the system call if available, otherwise use the > socket call. > * sysdeps/unix/sysv/linux/ia64/configure.ac (arch_minimum_kernel): > Set to 3.2.18. > * sysdeps/unix/sysv/linux/ia64/kernel-features.h > (__ASSUME_ACCEPT4_SYSCALL, __ASSUME_ACCEPT4): Do not undefine. > accept4 is unconditionally available in later 3.2 stable kernels. > (__ASSUME_ACCEPT4_SYSCALL): Define. > * sysdeps/unix/sysv/linux/kernel-features.h > (__ASSUME_ACCEPT4_SOCKETCALL, __ASSUME_ACCEPT4): Remove > definitions. > * sysdeps/unix/sysv/linux/i386/kernel-features.h > (__ASSUME_ACCEPT4_SYSCALL): Define for Linux 4.3 or later. > > diff --git a/INSTALL b/INSTALL > index 60f714e..920c4df 100644 > --- a/INSTALL > +++ b/INSTALL > @@ -501,13 +501,15 @@ Specific advice for GNU/Linux systems > > If you are installing the GNU C Library on GNU/Linux systems, you need > to have the header files from a 3.2 or newer kernel around for > -reference. These headers must be installed using 'make > -headers_install'; the headers present in the kernel source directory are > -not suitable for direct use by the GNU C Library. You do not need to > -use that kernel, just have its headers installed where the GNU C Library > -can access them, referred to here as INSTALL-DIRECTORY. The easiest way > -to do this is to unpack it in a directory such as > -'/usr/src/linux-VERSION'. In that directory, run 'make headers_install > +reference. (For the ia64 architecture, you need version 3.2.18 or newer > +because this is the first version with support for the 'accept4' system > +call.) These headers must be installed using 'make headers_install'; > +the headers present in the kernel source directory are not suitable for > +direct use by the GNU C Library. You do not need to use that kernel, > +just have its headers installed where the GNU C Library can access them, > +referred to here as INSTALL-DIRECTORY. The easiest way to do this is to > +unpack it in a directory such as '/usr/src/linux-VERSION'. In that > +directory, run 'make headers_install > INSTALL_HDR_PATH=INSTALL-DIRECTORY'. Finally, configure the GNU C > Library with the option '--with-headers=INSTALL-DIRECTORY/include'. Use > the most recent kernel you can get your hands on. (If you are > diff --git a/manual/install.texi b/manual/install.texi > index 99397c2..d39d2da 100644 > --- a/manual/install.texi > +++ b/manual/install.texi > @@ -566,6 +566,8 @@ patches, although we try to avoid this. > > If you are installing @theglibc{} on @gnulinuxsystems{}, you need to have > the header files from a 3.2 or newer kernel around for reference. > +(For the ia64 architecture, you need version 3.2.18 or newer because this > +is the first version with support for the @code{accept4} system call.) > These headers must be installed using @samp{make headers_install}; the > headers present in the kernel source directory are not suitable for > direct use by @theglibc{}. You do not need to use that kernel, just have > diff --git a/nscd/connections.c b/nscd/connections.c > index a5ca57a..cc1ed72 100644 > --- a/nscd/connections.c > +++ b/nscd/connections.c > @@ -257,10 +257,6 @@ int inotify_fd = -1; > static int nl_status_fd = -1; > #endif > > -#ifndef __ASSUME_ACCEPT4 > -static int have_accept4; > -#endif > - > /* Number of times clients had to wait. */ > unsigned long int client_queued; > > @@ -1650,16 +1646,6 @@ nscd_run_worker (void *p) > /* We are done with the list. */ > pthread_mutex_unlock (&readylist_lock); > > -#ifndef __ASSUME_ACCEPT4 > - if (have_accept4 < 0) > - { > - /* We do not want to block on a short read or so. */ > - int fl = fcntl (fd, F_GETFL); > - if (fl == -1 || fcntl (fd, F_SETFL, fl | O_NONBLOCK) == -1) > - goto close_and_out; > - } > -#endif > - > /* Now read the request. */ > request_header req; > if (__builtin_expect (TEMP_FAILURE_RETRY (read (fd, &req, sizeof (req))) > @@ -2099,24 +2085,8 @@ main_loop_poll (void) > if (conns[0].revents != 0) > { > /* We have a new incoming connection. Accept the connection. */ > - int fd; > - > -#ifndef __ASSUME_ACCEPT4 > - fd = -1; > - if (have_accept4 >= 0) > -#endif > - { > - fd = TEMP_FAILURE_RETRY (accept4 (sock, NULL, NULL, > + int fd = TEMP_FAILURE_RETRY (accept4 (sock, NULL, NULL, > SOCK_NONBLOCK)); > -#ifndef __ASSUME_ACCEPT4 > - if (have_accept4 == 0) > - have_accept4 = fd != -1 || errno != ENOSYS ? 1 : -1; > -#endif > - } > -#ifndef __ASSUME_ACCEPT4 > - if (have_accept4 < 0) > - fd = TEMP_FAILURE_RETRY (accept (sock, NULL, NULL)); > -#endif > > /* Use the descriptor if we have not reached the limit. */ > if (fd >= 0) > @@ -2284,24 +2254,8 @@ main_loop_epoll (int efd) > if (revs[cnt].data.fd == sock) > { > /* A new connection. */ > - int fd; > - > -# ifndef __ASSUME_ACCEPT4 > - fd = -1; > - if (have_accept4 >= 0) > -# endif > - { > - fd = TEMP_FAILURE_RETRY (accept4 (sock, NULL, NULL, > + int fd = TEMP_FAILURE_RETRY (accept4 (sock, NULL, NULL, > SOCK_NONBLOCK)); > -# ifndef __ASSUME_ACCEPT4 > - if (have_accept4 == 0) > - have_accept4 = fd != -1 || errno != ENOSYS ? 1 : -1; > -# endif > - } > -# ifndef __ASSUME_ACCEPT4 > - if (have_accept4 < 0) > - fd = TEMP_FAILURE_RETRY (accept (sock, NULL, NULL)); > -# endif > > /* Use the descriptor if we have not reached the limit. */ > if (fd >= 0) > diff --git a/socket/Makefile b/socket/Makefile > index 25d4f68..1e2555d 100644 > --- a/socket/Makefile > +++ b/socket/Makefile > @@ -31,6 +31,8 @@ routines := accept bind connect getpeername getsockname getsockopt \ > setsockopt shutdown socket socketpair isfdtype opensock \ > sockatmark accept4 recvmmsg sendmmsg > > +tests := tst-accept4 > + > aux := sa_len > > include ../Rules > diff --git a/socket/tst-accept4.c b/socket/tst-accept4.c > new file mode 100644 > index 0000000..a5cf3a2 > --- /dev/null > +++ b/socket/tst-accept4.c > @@ -0,0 +1,131 @@ > +/* Test the accept4 function with differing flags arguments. > + Copyright (C) 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 <arpa/inet.h> > +#include <errno.h> > +#include <fcntl.h> > +#include <stdbool.h> > +#include <support/check.h> > +#include <support/xsocket.h> > +#include <support/xunistd.h> > +#include <sys/socket.h> > + > +static bool > +is_nonblocking (int fd) > +{ > + int status = fcntl (fd, F_GETFL); > + if (status < 0) > + FAIL_EXIT1 ("fcntl (F_GETFL): %m"); > + return status & O_NONBLOCK; > +} > + > +static bool > +is_cloexec (int fd) > +{ > + int status = fcntl (fd, F_GETFD); > + if (status < 0) > + FAIL_EXIT1 ("fcntl (F_GETFD): %m"); > + return status & FD_CLOEXEC; > +} > + > +struct client > +{ > + int socket; > + struct sockaddr_in address; > +}; > + > +/* Perform a non-blocking connect to *SERVER_ADDRESS. */ > +static struct client > +client_connect (const struct sockaddr_in *server_address) > +{ > + struct client result; > + result.socket = xsocket (AF_INET, > + SOCK_STREAM | SOCK_NONBLOCK | SOCK_CLOEXEC, 0); > + TEST_VERIFY (is_nonblocking (result.socket)); > + TEST_VERIFY (is_cloexec (result.socket)); > + int ret = connect (result.socket, (const struct sockaddr *) server_address, > + sizeof (*server_address)); > + if (ret < 0 && errno != EINPROGRESS) > + FAIL_EXIT1 ("client connect: %m"); > + socklen_t sa_len = sizeof (result.address); > + xgetsockname (result.socket, (struct sockaddr *) &result.address, > + &sa_len); > + TEST_VERIFY (sa_len == sizeof (result.address)); > + return result; > +} > + > +static void > +check_same_address (const struct sockaddr_in *left, > + const struct sockaddr_in *right) > +{ > + TEST_VERIFY (left->sin_family == AF_INET); > + TEST_VERIFY (right->sin_family == AF_INET); > + TEST_VERIFY (left->sin_addr.s_addr == right->sin_addr.s_addr); > + TEST_VERIFY (left->sin_port == right->sin_port); > +} > + > +static int > +do_test (void) > +{ > + /* Create server socket. */ > + int server_socket = xsocket (AF_INET, SOCK_STREAM, 0); > + TEST_VERIFY (!is_nonblocking (server_socket)); > + TEST_VERIFY (!is_cloexec (server_socket)); > + struct sockaddr_in server_address = > + { > + .sin_family = AF_INET, > + .sin_addr = {.s_addr = htonl (INADDR_LOOPBACK) }, > + }; > + xbind (server_socket, > + (struct sockaddr *) &server_address, sizeof (server_address)); > + { > + socklen_t sa_len = sizeof (server_address); > + xgetsockname (server_socket, (struct sockaddr *) &server_address, > + &sa_len); > + TEST_VERIFY (sa_len == sizeof (server_address)); > + } > + xlisten (server_socket, 5); > + > + for (int do_nonblock = 0; do_nonblock < 2; ++do_nonblock) > + for (int do_cloexec = 0; do_cloexec < 2; ++do_cloexec) > + { > + int sockflags = 0; > + if (do_nonblock) > + sockflags |= SOCK_NONBLOCK; > + if (do_cloexec) > + sockflags |= SOCK_CLOEXEC; > + > + struct client client = client_connect (&server_address); > + struct sockaddr_in client_address; > + socklen_t sa_len = sizeof (client_address); > + int client_socket = xaccept4 (server_socket, > + (struct sockaddr *) &client_address, > + &sa_len, sockflags); > + TEST_VERIFY (sa_len == sizeof (client_address)); > + TEST_VERIFY (is_nonblocking (client_socket) == do_nonblock); > + TEST_VERIFY (is_cloexec (client_socket) == do_cloexec); > + check_same_address (&client.address, &client_address); > + xclose (client_socket); > + xclose (client.socket); > + } > + > + xclose (server_socket); > + return 0; > +} > + > +#include <support/test-driver.c> > diff --git a/support/Makefile b/support/Makefile > index 1f33fa0..38dbd83 100644 > --- a/support/Makefile > +++ b/support/Makefile > @@ -50,6 +50,7 @@ libsupport-routines = \ > temp_file \ > write_message \ > xaccept \ > + xaccept4 \ > xasprintf \ > xbind \ > xcalloc \ > diff --git a/support/xaccept4.c b/support/xaccept4.c > new file mode 100644 > index 0000000..67dd95e > --- /dev/null > +++ b/support/xaccept4.c > @@ -0,0 +1,32 @@ > +/* accept4 with error checking. > + Copyright (C) 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 <support/xsocket.h> > + > +#include <stdio.h> > +#include <stdlib.h> > +#include <support/check.h> > + > +int > +xaccept4 (int fd, struct sockaddr *sa, socklen_t *salen, int flags) > +{ > + int clientfd = accept4 (fd, sa, salen, flags); > + if (clientfd < 0) > + FAIL_EXIT1 ("accept4 (%d, 0x%x): %m", fd, flags); > + return clientfd; > +} > diff --git a/support/xsocket.h b/support/xsocket.h > index 0dbf13a..d672494 100644 > --- a/support/xsocket.h > +++ b/support/xsocket.h > @@ -30,6 +30,7 @@ void xconnect (int, const struct sockaddr *, socklen_t); > void xbind (int, const struct sockaddr *, socklen_t); > void xlisten (int, int); > int xaccept (int, struct sockaddr *, socklen_t *); > +int xaccept4 (int, struct sockaddr *, socklen_t *, int); > void xsendto (int, const void *, size_t, int, > const struct sockaddr *, socklen_t); > size_t xrecvfrom (int, void *, size_t, int, struct sockaddr *, socklen_t *); > diff --git a/sysdeps/mach/hurd/kernel-features.h b/sysdeps/mach/hurd/kernel-features.h > index 20cb3ba..1966388 100644 > --- a/sysdeps/mach/hurd/kernel-features.h > +++ b/sysdeps/mach/hurd/kernel-features.h > @@ -19,5 +19,3 @@ > /* This file can define __ASSUME_* macros checked by certain source files. > Almost none of these are used outside of sysdeps/unix/sysv/linux code. > But those referring to POSIX-level features like O_* flags can be. */ > - > -#define __ASSUME_ACCEPT4 1 > diff --git a/sysdeps/nacl/kernel-features.h b/sysdeps/nacl/kernel-features.h > index 32834fd..ba315b6 100644 > --- a/sysdeps/nacl/kernel-features.h > +++ b/sysdeps/nacl/kernel-features.h > @@ -19,7 +19,3 @@ > /* This file can define __ASSUME_* macros checked by certain source files. > Almost none of these are used outside of sysdeps/unix/sysv/linux code. > But those referring to POSIX-level features like O_* flags can be. */ > - > -/* > -#define __ASSUME_ACCEPT4 1 > -*/ > diff --git a/sysdeps/unix/sysv/linux/accept4.c b/sysdeps/unix/sysv/linux/accept4.c > index 53afd7a..0592f43 100644 > --- a/sysdeps/unix/sysv/linux/accept4.c > +++ b/sysdeps/unix/sysv/linux/accept4.c > @@ -22,75 +22,20 @@ > > #include <sysdep-cancel.h> > #include <sys/syscall.h> > +#include <socketcall.h> > #include <kernel-features.h> > > +int > +accept4 (int fd, __SOCKADDR_ARG addr, socklen_t *addr_len, int flags) > +{ > /* Do not use the accept4 syscall on socketcall architectures unless > it was added at the same time as the socketcall support or can be > assumed to be present. */ > #if defined __ASSUME_SOCKETCALL \ > && !defined __ASSUME_ACCEPT4_SYSCALL_WITH_SOCKETCALL \ > && !defined __ASSUME_ACCEPT4_SYSCALL > -# undef __NR_accept4 > -#endif > - > -#ifdef __NR_accept4 > -int > -accept4 (int fd, __SOCKADDR_ARG addr, socklen_t *addr_len, int flags) > -{ > - return SYSCALL_CANCEL (accept4, fd, addr.__sockaddr__, addr_len, flags); > -} > -#elif defined __NR_socketcall > -# include <socketcall.h> > -# ifdef __ASSUME_ACCEPT4_SOCKETCALL > -int > -accept4 (int fd, __SOCKADDR_ARG addr, socklen_t *addr_len, int flags) > -{ > return SOCKETCALL_CANCEL (accept4, fd, addr.__sockaddr__, addr_len, flags); > -} > -# else > -static int have_accept4; > - > -int > -accept4 (int fd, __SOCKADDR_ARG addr, socklen_t *addr_len, int flags) > -{ > - if (__glibc_likely (have_accept4 >= 0)) > - { > - int ret = SOCKETCALL_CANCEL (accept4, fd, addr.__sockaddr__, addr_len, > - flags); > - /* The kernel returns -EINVAL for unknown socket operations. > - We need to convert that error to an ENOSYS error. */ > - if (__builtin_expect (ret < 0, 0) > - && have_accept4 == 0 > - && errno == EINVAL) > - { > - /* Try another call, this time with the FLAGS parameter > - cleared and an invalid file descriptor. This call will not > - cause any harm and it will return immediately. */ > - ret = SOCKETCALL_CANCEL (invalid, -1); > - if (errno == EINVAL) > - { > - have_accept4 = -1; > - __set_errno (ENOSYS); > - } > - else > - { > - have_accept4 = 1; > - __set_errno (EINVAL); > - } > - return -1; > - } > - return ret; > - } > - __set_errno (ENOSYS); > - return -1; > -} > -# endif /* __ASSUME_ACCEPT4_SOCKETCALL */ > -#else /* __NR_socketcall */ > -int > -accept4 (int fd, __SOCKADDR_ARG addr, socklen_t *addr_len, int flags) > -{ > - __set_errno (ENOSYS); > - return -1; > -} > -stub_warning (accept4) > +#else > + return SYSCALL_CANCEL (accept4, fd, addr.__sockaddr__, addr_len, flags); > #endif > +} > diff --git a/sysdeps/unix/sysv/linux/ia64/configure b/sysdeps/unix/sysv/linux/ia64/configure > index 1d4e5d1..3cf7237 100644 > --- a/sysdeps/unix/sysv/linux/ia64/configure > +++ b/sysdeps/unix/sysv/linux/ia64/configure > @@ -2,3 +2,7 @@ > # Local configure fragment for sysdeps/unix/sysv/linux/ia64 > > ldd_rewrite_script=$dir/ldd-rewrite.sed > + > +# First version with support for the accept4 system call. > +# Linux 3.3 includes it as well. > +arch_minimum_kernel=3.2.18 > diff --git a/sysdeps/unix/sysv/linux/ia64/configure.ac b/sysdeps/unix/sysv/linux/ia64/configure.ac > index 4fb5647..94a578c 100644 > --- a/sysdeps/unix/sysv/linux/ia64/configure.ac > +++ b/sysdeps/unix/sysv/linux/ia64/configure.ac > @@ -2,3 +2,7 @@ GLIBC_PROVIDES dnl See aclocal.m4 in the top level source directory. > # Local configure fragment for sysdeps/unix/sysv/linux/ia64 > > ldd_rewrite_script=$dir/ldd-rewrite.sed > + > +# First version with support for the accept4 system call. > +# Linux 3.3 includes it as well. > +arch_minimum_kernel=3.2.18 > diff --git a/sysdeps/unix/sysv/linux/ia64/kernel-features.h b/sysdeps/unix/sysv/linux/ia64/kernel-features.h > index ac9403e..cda0ad6 100644 > --- a/sysdeps/unix/sysv/linux/ia64/kernel-features.h > +++ b/sysdeps/unix/sysv/linux/ia64/kernel-features.h > @@ -22,13 +22,8 @@ > > #include_next <kernel-features.h> > > -/* Support for the accept4 syscall was added in 3.3. */ > -#if __LINUX_KERNEL_VERSION < 0x030300 > -# undef __ASSUME_ACCEPT4_SYSCALL > -# undef __ASSUME_ACCEPT4 > -#endif > - > #define __ASSUME_RECV_SYSCALL 1 > #define __ASSUME_SEND_SYSCALL 1 > +#define __ASSUME_ACCEPT4_SYSCALL 1 > > #endif /* _KERNEL_FEATURES_H */ > diff --git a/sysdeps/unix/sysv/linux/kernel-features.h b/sysdeps/unix/sysv/linux/kernel-features.h > index fd936c5..b981466 100644 > --- a/sysdeps/unix/sysv/linux/kernel-features.h > +++ b/sysdeps/unix/sysv/linux/kernel-features.h > @@ -75,18 +75,11 @@ > architectures using a separate syscall rather than socketcall that > syscall was only added later, and some architectures first had > socketcall support then a separate syscall. Define > - __ASSUME_ACCEPT4_SOCKETCALL if glibc uses socketcall on this > - architecture and accept4 is available through socketcall, > __ASSUME_ACCEPT4_SYSCALL if it is available through a separate > - syscall, __ASSUME_ACCEPT4_SYSCALL_WITH_SOCKETCALL if it became > + syscall, and __ASSUME_ACCEPT4_SYSCALL_WITH_SOCKETCALL if it became > available through a separate syscall at the same time as through > - socketcall, and __ASSUME_ACCEPT4 if the accept4 function is known > - to work. */ > -#ifdef __ASSUME_SOCKETCALL > -# define __ASSUME_ACCEPT4_SOCKETCALL 1 > -#endif > + socketcall. */ > #define __ASSUME_ACCEPT4_SYSCALL 1 > -#define __ASSUME_ACCEPT4 1 > > /* Support for the FUTEX_CLOCK_REALTIME flag was added in 2.6.29. */ > #define __ASSUME_FUTEX_CLOCK_REALTIME 1 >
Assume that accept4 is always available and works Simplify the Linux accept4 implementation based on the assumption that it is available in some way. __ASSUME_ACCEPT4_SOCKETCALL was previously unused, so remove it. For ia64, the accept4 system call (and socket call) were backported in kernel version 3.2.18. Reflect this in the installation instructions. 2017-04-18 Florian Weimer <fweimer@redhat.com> * nscd/connections.c (have_accept4): Removed definition. (nscd_run_worker, main_loop_poll, main_loop_epolll): Assume that accept4 works. * manual/install.texi (Linux): Require at least kernel 3.2.18 for ia64 because that was the first version with accept4 support. * support/Makefile (libsupport-routines): Add xaccept4. * support/xsocket.h (xaccept4): Declare. * support/xaccept4.c: New file. * socket/tst-accept4.c: New file. * socket/Makefile (tests): Add tst-accept4. * sysdeps/mach/hurd/kernel-features.h (__ASSUME_ACCEPT4): Remove definition. * sysdeps/nacl/kernel-features.h (__ASSUME_ACCEPT4): Remove comment. * sysdeps/unix/sysv/linux/accept4.c: Assume that an accept4 implementation is available. (accept4): Use the system call if available, otherwise use the socket call. * sysdeps/unix/sysv/linux/ia64/configure.ac (arch_minimum_kernel): Set to 3.2.18. * sysdeps/unix/sysv/linux/ia64/kernel-features.h (__ASSUME_ACCEPT4_SYSCALL, __ASSUME_ACCEPT4): Do not undefine. accept4 is unconditionally available in later 3.2 stable kernels. (__ASSUME_ACCEPT4_SYSCALL): Define. * sysdeps/unix/sysv/linux/kernel-features.h (__ASSUME_ACCEPT4_SOCKETCALL, __ASSUME_ACCEPT4): Remove definitions. * sysdeps/unix/sysv/linux/i386/kernel-features.h (__ASSUME_ACCEPT4_SYSCALL): Define for Linux 4.3 or later. diff --git a/INSTALL b/INSTALL index 60f714e..920c4df 100644 --- a/INSTALL +++ b/INSTALL @@ -501,13 +501,15 @@ Specific advice for GNU/Linux systems If you are installing the GNU C Library on GNU/Linux systems, you need to have the header files from a 3.2 or newer kernel around for -reference. These headers must be installed using 'make -headers_install'; the headers present in the kernel source directory are -not suitable for direct use by the GNU C Library. You do not need to -use that kernel, just have its headers installed where the GNU C Library -can access them, referred to here as INSTALL-DIRECTORY. The easiest way -to do this is to unpack it in a directory such as -'/usr/src/linux-VERSION'. In that directory, run 'make headers_install +reference. (For the ia64 architecture, you need version 3.2.18 or newer +because this is the first version with support for the 'accept4' system +call.) These headers must be installed using 'make headers_install'; +the headers present in the kernel source directory are not suitable for +direct use by the GNU C Library. You do not need to use that kernel, +just have its headers installed where the GNU C Library can access them, +referred to here as INSTALL-DIRECTORY. The easiest way to do this is to +unpack it in a directory such as '/usr/src/linux-VERSION'. In that +directory, run 'make headers_install INSTALL_HDR_PATH=INSTALL-DIRECTORY'. Finally, configure the GNU C Library with the option '--with-headers=INSTALL-DIRECTORY/include'. Use the most recent kernel you can get your hands on. (If you are diff --git a/manual/install.texi b/manual/install.texi index 99397c2..d39d2da 100644 --- a/manual/install.texi +++ b/manual/install.texi @@ -566,6 +566,8 @@ patches, although we try to avoid this. If you are installing @theglibc{} on @gnulinuxsystems{}, you need to have the header files from a 3.2 or newer kernel around for reference. +(For the ia64 architecture, you need version 3.2.18 or newer because this +is the first version with support for the @code{accept4} system call.) These headers must be installed using @samp{make headers_install}; the headers present in the kernel source directory are not suitable for direct use by @theglibc{}. You do not need to use that kernel, just have diff --git a/nscd/connections.c b/nscd/connections.c index a5ca57a..cc1ed72 100644 --- a/nscd/connections.c +++ b/nscd/connections.c @@ -257,10 +257,6 @@ int inotify_fd = -1; static int nl_status_fd = -1; #endif -#ifndef __ASSUME_ACCEPT4 -static int have_accept4; -#endif - /* Number of times clients had to wait. */ unsigned long int client_queued; @@ -1650,16 +1646,6 @@ nscd_run_worker (void *p) /* We are done with the list. */ pthread_mutex_unlock (&readylist_lock); -#ifndef __ASSUME_ACCEPT4 - if (have_accept4 < 0) - { - /* We do not want to block on a short read or so. */ - int fl = fcntl (fd, F_GETFL); - if (fl == -1 || fcntl (fd, F_SETFL, fl | O_NONBLOCK) == -1) - goto close_and_out; - } -#endif - /* Now read the request. */ request_header req; if (__builtin_expect (TEMP_FAILURE_RETRY (read (fd, &req, sizeof (req))) @@ -2099,24 +2085,8 @@ main_loop_poll (void) if (conns[0].revents != 0) { /* We have a new incoming connection. Accept the connection. */ - int fd; - -#ifndef __ASSUME_ACCEPT4 - fd = -1; - if (have_accept4 >= 0) -#endif - { - fd = TEMP_FAILURE_RETRY (accept4 (sock, NULL, NULL, + int fd = TEMP_FAILURE_RETRY (accept4 (sock, NULL, NULL, SOCK_NONBLOCK)); -#ifndef __ASSUME_ACCEPT4 - if (have_accept4 == 0) - have_accept4 = fd != -1 || errno != ENOSYS ? 1 : -1; -#endif - } -#ifndef __ASSUME_ACCEPT4 - if (have_accept4 < 0) - fd = TEMP_FAILURE_RETRY (accept (sock, NULL, NULL)); -#endif /* Use the descriptor if we have not reached the limit. */ if (fd >= 0) @@ -2284,24 +2254,8 @@ main_loop_epoll (int efd) if (revs[cnt].data.fd == sock) { /* A new connection. */ - int fd; - -# ifndef __ASSUME_ACCEPT4 - fd = -1; - if (have_accept4 >= 0) -# endif - { - fd = TEMP_FAILURE_RETRY (accept4 (sock, NULL, NULL, + int fd = TEMP_FAILURE_RETRY (accept4 (sock, NULL, NULL, SOCK_NONBLOCK)); -# ifndef __ASSUME_ACCEPT4 - if (have_accept4 == 0) - have_accept4 = fd != -1 || errno != ENOSYS ? 1 : -1; -# endif - } -# ifndef __ASSUME_ACCEPT4 - if (have_accept4 < 0) - fd = TEMP_FAILURE_RETRY (accept (sock, NULL, NULL)); -# endif /* Use the descriptor if we have not reached the limit. */ if (fd >= 0) diff --git a/socket/Makefile b/socket/Makefile index 25d4f68..1e2555d 100644 --- a/socket/Makefile +++ b/socket/Makefile @@ -31,6 +31,8 @@ routines := accept bind connect getpeername getsockname getsockopt \ setsockopt shutdown socket socketpair isfdtype opensock \ sockatmark accept4 recvmmsg sendmmsg +tests := tst-accept4 + aux := sa_len include ../Rules diff --git a/socket/tst-accept4.c b/socket/tst-accept4.c new file mode 100644 index 0000000..a5cf3a2 --- /dev/null +++ b/socket/tst-accept4.c @@ -0,0 +1,131 @@ +/* Test the accept4 function with differing flags arguments. + Copyright (C) 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 <arpa/inet.h> +#include <errno.h> +#include <fcntl.h> +#include <stdbool.h> +#include <support/check.h> +#include <support/xsocket.h> +#include <support/xunistd.h> +#include <sys/socket.h> + +static bool +is_nonblocking (int fd) +{ + int status = fcntl (fd, F_GETFL); + if (status < 0) + FAIL_EXIT1 ("fcntl (F_GETFL): %m"); + return status & O_NONBLOCK; +} + +static bool +is_cloexec (int fd) +{ + int status = fcntl (fd, F_GETFD); + if (status < 0) + FAIL_EXIT1 ("fcntl (F_GETFD): %m"); + return status & FD_CLOEXEC; +} + +struct client +{ + int socket; + struct sockaddr_in address; +}; + +/* Perform a non-blocking connect to *SERVER_ADDRESS. */ +static struct client +client_connect (const struct sockaddr_in *server_address) +{ + struct client result; + result.socket = xsocket (AF_INET, + SOCK_STREAM | SOCK_NONBLOCK | SOCK_CLOEXEC, 0); + TEST_VERIFY (is_nonblocking (result.socket)); + TEST_VERIFY (is_cloexec (result.socket)); + int ret = connect (result.socket, (const struct sockaddr *) server_address, + sizeof (*server_address)); + if (ret < 0 && errno != EINPROGRESS) + FAIL_EXIT1 ("client connect: %m"); + socklen_t sa_len = sizeof (result.address); + xgetsockname (result.socket, (struct sockaddr *) &result.address, + &sa_len); + TEST_VERIFY (sa_len == sizeof (result.address)); + return result; +} + +static void +check_same_address (const struct sockaddr_in *left, + const struct sockaddr_in *right) +{ + TEST_VERIFY (left->sin_family == AF_INET); + TEST_VERIFY (right->sin_family == AF_INET); + TEST_VERIFY (left->sin_addr.s_addr == right->sin_addr.s_addr); + TEST_VERIFY (left->sin_port == right->sin_port); +} + +static int +do_test (void) +{ + /* Create server socket. */ + int server_socket = xsocket (AF_INET, SOCK_STREAM, 0); + TEST_VERIFY (!is_nonblocking (server_socket)); + TEST_VERIFY (!is_cloexec (server_socket)); + struct sockaddr_in server_address = + { + .sin_family = AF_INET, + .sin_addr = {.s_addr = htonl (INADDR_LOOPBACK) }, + }; + xbind (server_socket, + (struct sockaddr *) &server_address, sizeof (server_address)); + { + socklen_t sa_len = sizeof (server_address); + xgetsockname (server_socket, (struct sockaddr *) &server_address, + &sa_len); + TEST_VERIFY (sa_len == sizeof (server_address)); + } + xlisten (server_socket, 5); + + for (int do_nonblock = 0; do_nonblock < 2; ++do_nonblock) + for (int do_cloexec = 0; do_cloexec < 2; ++do_cloexec) + { + int sockflags = 0; + if (do_nonblock) + sockflags |= SOCK_NONBLOCK; + if (do_cloexec) + sockflags |= SOCK_CLOEXEC; + + struct client client = client_connect (&server_address); + struct sockaddr_in client_address; + socklen_t sa_len = sizeof (client_address); + int client_socket = xaccept4 (server_socket, + (struct sockaddr *) &client_address, + &sa_len, sockflags); + TEST_VERIFY (sa_len == sizeof (client_address)); + TEST_VERIFY (is_nonblocking (client_socket) == do_nonblock); + TEST_VERIFY (is_cloexec (client_socket) == do_cloexec); + check_same_address (&client.address, &client_address); + xclose (client_socket); + xclose (client.socket); + } + + xclose (server_socket); + return 0; +} + +#include <support/test-driver.c> diff --git a/support/Makefile b/support/Makefile index 1f33fa0..38dbd83 100644 --- a/support/Makefile +++ b/support/Makefile @@ -50,6 +50,7 @@ libsupport-routines = \ temp_file \ write_message \ xaccept \ + xaccept4 \ xasprintf \ xbind \ xcalloc \ diff --git a/support/xaccept4.c b/support/xaccept4.c new file mode 100644 index 0000000..67dd95e --- /dev/null +++ b/support/xaccept4.c @@ -0,0 +1,32 @@ +/* accept4 with error checking. + Copyright (C) 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 <support/xsocket.h> + +#include <stdio.h> +#include <stdlib.h> +#include <support/check.h> + +int +xaccept4 (int fd, struct sockaddr *sa, socklen_t *salen, int flags) +{ + int clientfd = accept4 (fd, sa, salen, flags); + if (clientfd < 0) + FAIL_EXIT1 ("accept4 (%d, 0x%x): %m", fd, flags); + return clientfd; +} diff --git a/support/xsocket.h b/support/xsocket.h index 0dbf13a..d672494 100644 --- a/support/xsocket.h +++ b/support/xsocket.h @@ -30,6 +30,7 @@ void xconnect (int, const struct sockaddr *, socklen_t); void xbind (int, const struct sockaddr *, socklen_t); void xlisten (int, int); int xaccept (int, struct sockaddr *, socklen_t *); +int xaccept4 (int, struct sockaddr *, socklen_t *, int); void xsendto (int, const void *, size_t, int, const struct sockaddr *, socklen_t); size_t xrecvfrom (int, void *, size_t, int, struct sockaddr *, socklen_t *); diff --git a/sysdeps/mach/hurd/kernel-features.h b/sysdeps/mach/hurd/kernel-features.h index 20cb3ba..1966388 100644 --- a/sysdeps/mach/hurd/kernel-features.h +++ b/sysdeps/mach/hurd/kernel-features.h @@ -19,5 +19,3 @@ /* This file can define __ASSUME_* macros checked by certain source files. Almost none of these are used outside of sysdeps/unix/sysv/linux code. But those referring to POSIX-level features like O_* flags can be. */ - -#define __ASSUME_ACCEPT4 1 diff --git a/sysdeps/nacl/kernel-features.h b/sysdeps/nacl/kernel-features.h index 32834fd..ba315b6 100644 --- a/sysdeps/nacl/kernel-features.h +++ b/sysdeps/nacl/kernel-features.h @@ -19,7 +19,3 @@ /* This file can define __ASSUME_* macros checked by certain source files. Almost none of these are used outside of sysdeps/unix/sysv/linux code. But those referring to POSIX-level features like O_* flags can be. */ - -/* -#define __ASSUME_ACCEPT4 1 -*/ diff --git a/sysdeps/unix/sysv/linux/accept4.c b/sysdeps/unix/sysv/linux/accept4.c index 53afd7a..0592f43 100644 --- a/sysdeps/unix/sysv/linux/accept4.c +++ b/sysdeps/unix/sysv/linux/accept4.c @@ -22,75 +22,20 @@ #include <sysdep-cancel.h> #include <sys/syscall.h> +#include <socketcall.h> #include <kernel-features.h> +int +accept4 (int fd, __SOCKADDR_ARG addr, socklen_t *addr_len, int flags) +{ /* Do not use the accept4 syscall on socketcall architectures unless it was added at the same time as the socketcall support or can be assumed to be present. */ #if defined __ASSUME_SOCKETCALL \ && !defined __ASSUME_ACCEPT4_SYSCALL_WITH_SOCKETCALL \ && !defined __ASSUME_ACCEPT4_SYSCALL -# undef __NR_accept4 -#endif - -#ifdef __NR_accept4 -int -accept4 (int fd, __SOCKADDR_ARG addr, socklen_t *addr_len, int flags) -{ - return SYSCALL_CANCEL (accept4, fd, addr.__sockaddr__, addr_len, flags); -} -#elif defined __NR_socketcall -# include <socketcall.h> -# ifdef __ASSUME_ACCEPT4_SOCKETCALL -int -accept4 (int fd, __SOCKADDR_ARG addr, socklen_t *addr_len, int flags) -{ return SOCKETCALL_CANCEL (accept4, fd, addr.__sockaddr__, addr_len, flags); -} -# else -static int have_accept4; - -int -accept4 (int fd, __SOCKADDR_ARG addr, socklen_t *addr_len, int flags) -{ - if (__glibc_likely (have_accept4 >= 0)) - { - int ret = SOCKETCALL_CANCEL (accept4, fd, addr.__sockaddr__, addr_len, - flags); - /* The kernel returns -EINVAL for unknown socket operations. - We need to convert that error to an ENOSYS error. */ - if (__builtin_expect (ret < 0, 0) - && have_accept4 == 0 - && errno == EINVAL) - { - /* Try another call, this time with the FLAGS parameter - cleared and an invalid file descriptor. This call will not - cause any harm and it will return immediately. */ - ret = SOCKETCALL_CANCEL (invalid, -1); - if (errno == EINVAL) - { - have_accept4 = -1; - __set_errno (ENOSYS); - } - else - { - have_accept4 = 1; - __set_errno (EINVAL); - } - return -1; - } - return ret; - } - __set_errno (ENOSYS); - return -1; -} -# endif /* __ASSUME_ACCEPT4_SOCKETCALL */ -#else /* __NR_socketcall */ -int -accept4 (int fd, __SOCKADDR_ARG addr, socklen_t *addr_len, int flags) -{ - __set_errno (ENOSYS); - return -1; -} -stub_warning (accept4) +#else + return SYSCALL_CANCEL (accept4, fd, addr.__sockaddr__, addr_len, flags); #endif +} diff --git a/sysdeps/unix/sysv/linux/ia64/configure b/sysdeps/unix/sysv/linux/ia64/configure index 1d4e5d1..3cf7237 100644 --- a/sysdeps/unix/sysv/linux/ia64/configure +++ b/sysdeps/unix/sysv/linux/ia64/configure @@ -2,3 +2,7 @@ # Local configure fragment for sysdeps/unix/sysv/linux/ia64 ldd_rewrite_script=$dir/ldd-rewrite.sed + +# First version with support for the accept4 system call. +# Linux 3.3 includes it as well. +arch_minimum_kernel=3.2.18 diff --git a/sysdeps/unix/sysv/linux/ia64/configure.ac b/sysdeps/unix/sysv/linux/ia64/configure.ac index 4fb5647..94a578c 100644 --- a/sysdeps/unix/sysv/linux/ia64/configure.ac +++ b/sysdeps/unix/sysv/linux/ia64/configure.ac @@ -2,3 +2,7 @@ GLIBC_PROVIDES dnl See aclocal.m4 in the top level source directory. # Local configure fragment for sysdeps/unix/sysv/linux/ia64 ldd_rewrite_script=$dir/ldd-rewrite.sed + +# First version with support for the accept4 system call. +# Linux 3.3 includes it as well. +arch_minimum_kernel=3.2.18 diff --git a/sysdeps/unix/sysv/linux/ia64/kernel-features.h b/sysdeps/unix/sysv/linux/ia64/kernel-features.h index ac9403e..cda0ad6 100644 --- a/sysdeps/unix/sysv/linux/ia64/kernel-features.h +++ b/sysdeps/unix/sysv/linux/ia64/kernel-features.h @@ -22,13 +22,8 @@ #include_next <kernel-features.h> -/* Support for the accept4 syscall was added in 3.3. */ -#if __LINUX_KERNEL_VERSION < 0x030300 -# undef __ASSUME_ACCEPT4_SYSCALL -# undef __ASSUME_ACCEPT4 -#endif - #define __ASSUME_RECV_SYSCALL 1 #define __ASSUME_SEND_SYSCALL 1 +#define __ASSUME_ACCEPT4_SYSCALL 1 #endif /* _KERNEL_FEATURES_H */ diff --git a/sysdeps/unix/sysv/linux/kernel-features.h b/sysdeps/unix/sysv/linux/kernel-features.h index fd936c5..b981466 100644 --- a/sysdeps/unix/sysv/linux/kernel-features.h +++ b/sysdeps/unix/sysv/linux/kernel-features.h @@ -75,18 +75,11 @@ architectures using a separate syscall rather than socketcall that syscall was only added later, and some architectures first had socketcall support then a separate syscall. Define - __ASSUME_ACCEPT4_SOCKETCALL if glibc uses socketcall on this - architecture and accept4 is available through socketcall, __ASSUME_ACCEPT4_SYSCALL if it is available through a separate - syscall, __ASSUME_ACCEPT4_SYSCALL_WITH_SOCKETCALL if it became + syscall, and __ASSUME_ACCEPT4_SYSCALL_WITH_SOCKETCALL if it became available through a separate syscall at the same time as through - socketcall, and __ASSUME_ACCEPT4 if the accept4 function is known - to work. */ -#ifdef __ASSUME_SOCKETCALL -# define __ASSUME_ACCEPT4_SOCKETCALL 1 -#endif + socketcall. */ #define __ASSUME_ACCEPT4_SYSCALL 1 -#define __ASSUME_ACCEPT4 1 /* Support for the FUTEX_CLOCK_REALTIME flag was added in 2.6.29. */ #define __ASSUME_FUTEX_CLOCK_REALTIME 1