From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 68353 invoked by alias); 17 Apr 2017 15:01:59 -0000 Mailing-List: contact libc-alpha-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: libc-alpha-owner@sourceware.org Received: (qmail 68343 invoked by uid 89); 17 Apr 2017 15:01:58 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-24.5 required=5.0 tests=AWL,BAYES_00,GIT_PATCH_0,GIT_PATCH_1,GIT_PATCH_2,GIT_PATCH_3,RCVD_IN_DNSWL_NONE,RCVD_IN_SORBS_SPAM,SPF_PASS autolearn=ham version=3.3.2 spammy=H*p:D*org, H*r:10.0.0 X-HELO: mail-qk0-f179.google.com X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:subject:to:references:from:message-id:date :user-agent:mime-version:in-reply-to:content-transfer-encoding; bh=jpZjzdSEBfZod/hWi3l4NX1Pxq7OI9iyhCJaJKPCZK4=; b=o5RmTrv3JbEcX6Gxuu4kXI1Hrn7R95mlQoKsAVe2pyvK8i9YuICVivRLPEs9vRmMw0 QR+acg3llirbtpfK+T3eX48LJeZi4u8kz0iHWvgtQqCndpg9Lbll+WWOYeRYHwNUuES3 sYIvhLZkuQFIoWEm6Kdj1j0vrIGyIfd0d4qT/JoKI0avoasDixYkS86mge+NsgJfa98N DNFC6fNR0vuXyAkBBaAC6Vwo4p3i3gGkNe6WZDvuroHuZ06SgwRr3vBqKbF2DYANrtdv rB7Y0ExxzXxvWDbXqAym5RTJuV/rkhJMd9+w5ohR5PpPRi7MgNKZAZT27JWIHw9cIAlq 28pQ== X-Gm-Message-State: AN3rC/6xaQGn3OGLOI6iSURVmcdCXHyiBGjjhiHOLY6WN7xlp6CFMF1t YXGmywusBSKQ6JmrxUvQLg== X-Received: by 10.233.237.194 with SMTP id c185mr8616139qkg.177.1492441314628; Mon, 17 Apr 2017 08:01:54 -0700 (PDT) Subject: Re: [PATCH] Assume that accept4 is available To: libc-alpha@sourceware.org References: <4ede44de-6607-025d-c816-a666d10db7dc@redhat.com> From: Adhemerval Zanella Message-ID: <89b754a4-b4b0-2499-6fed-f27742963620@linaro.org> Date: Mon, 17 Apr 2017 15:01:00 -0000 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.8.0 MIME-Version: 1.0 In-Reply-To: <4ede44de-6607-025d-c816-a666d10db7dc@redhat.com> Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit X-SW-Source: 2017-04/txt/msg00291.txt.bz2 This patch shows no regression on sparc64, armhf, and aarch64 as well. On 14/04/2017 07:45, Florian Weimer wrote: > diff --git a/INSTALL b/INSTALL > index 60f714e..51989b2 100644 > --- a/INSTALL > +++ b/INSTALL > @@ -500,21 +500,21 @@ 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 > -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 > -cross-compiling the GNU C Library, you need to specify > -'ARCH=ARCHITECTURE' in the 'make headers_install' command, where > -ARCHITECTURE is the architecture name used by the Linux kernel, such as > -'x86' or 'powerpc'.) > +to have the header files from a 3.2 or newer kernel (a 3.2.18 or newer > +kernel for the ia64 architecture) around for reference. These headers I think since you are referring a specific version for ia64, it's worth to add a reference why we are requiring it. Also, and more important, I think we need to reflect it on alpha 'arch_minimum_kernel' minimum version on configure.ac (so glibc configure fails to build with old installed kernel version). > diff --git a/socket/tst-accept4.c b/socket/tst-accept4.c > new file mode 100644 > index 0000000..0e02bc0 > --- /dev/null > +++ b/socket/tst-accept4.c > @@ -0,0 +1,130 @@ Missing line with file description. > +/* 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 > + . */ > + > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > + > +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)); > + } Why the brackets? I usually seems this unnecessary. > + 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 > 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..b676b58 > --- /dev/null > +++ b/support/xaccept4.c > @@ -0,0 +1,32 @@ > +/* accept with error checking. > + Copyright (C) 2016-2017 Free Software Foundation, Inc. I think it should be only 2017, shouldn't it? > + 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 > + . */ > + > +#include > + > +#include > +#include > +#include > + > +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 > #include > +#include > #include > > +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 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). > -# 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 > -# 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/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 > > -/* 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 >