Re: [PATCH] accept4 tests: fix specified flags

[Pádraig Brady]

On 08/10/15 13:47, Pino Toscano wrote:
> Pass only SOCK_* flags to accept4, as they are the only documented
> ones, and passing others may trigger EINVAL.
> * tests/test-accept4.c: (main): Pass SOCK_CLOEXEC instead of
> O_CLOEXEC | O_BINARY to accept4.
> ---
>  ChangeLog            | 7 +++++++
>  tests/test-accept4.c | 4 ++--
>  2 files changed, 9 insertions(+), 2 deletions(-)
> 
> diff --git a/ChangeLog b/ChangeLog
> index 02d8bf8..3601eda 100644
> --- a/ChangeLog
> +++ b/ChangeLog
> @@ -1,3 +1,10 @@
> +2015-10-08  Pino Toscano  <address@hidden>
> +
> +     Pass only SOCK_* flags to accept4, as they are the only documented
> +     ones, and passing others may trigger EINVAL.
> +     * tests/test-accept4.c: (main): Pass SOCK_CLOEXEC instead of
> +     O_CLOEXEC | O_BINARY to accept4.
> +
>  2015-10-06  Pavel Raiskup  <address@hidden>
>  
>       gnulib-tool: fix tests of 'extensions' module
> diff --git a/tests/test-accept4.c b/tests/test-accept4.c
> index b24af0b..b2e6fa8 100644
> --- a/tests/test-accept4.c
> +++ b/tests/test-accept4.c
> @@ -43,7 +43,7 @@ main (void)
>  
>      errno = 0;
>      ASSERT (accept4 (-1, (struct sockaddr *) &addr, &addrlen,
> -                     O_CLOEXEC | O_BINARY)
> +                     SOCK_CLOEXEC)
>              == -1);
>      ASSERT (errno == EBADF);
>    }
> @@ -54,7 +54,7 @@ main (void)
>      close (99);
>      errno = 0;
>      ASSERT (accept4 (99, (struct sockaddr *) &addr, &addrlen,
> -                     O_CLOEXEC | O_BINARY)
> +                     SOCK_CLOEXEC)
>              == -1);
>      ASSERT (errno == EBADF);
>    }

That change looks good, though it also suggests that
the implementation doesn't assume the availability of SOCK_CLOEXEC etc.
I think we also may need the following included in your patch,
to ensure the test compiles on all platforms, and that those
constants are defined appropriately on all platforms?

diff --git a/lib/accept4.c b/lib/accept4.c
index adf0989..992dfd0 100644
--- a/lib/accept4.c
+++ b/lib/accept4.c
@@ -24,10 +24,6 @@
 #include "binary-io.h"
 #include "msvc-nothrow.h"

-#ifndef SOCK_CLOEXEC
-# define SOCK_CLOEXEC 0
-#endif
-
 int
 accept4 (int sockfd, struct sockaddr *addr, socklen_t *addrlen, int flags)
 {
diff --git a/lib/sys_socket.in.h b/lib/sys_socket.in.h
index d29cc09..2d9df45 100644
--- a/lib/sys_socket.in.h
+++ b/lib/sys_socket.in.h
@@ -654,10 +654,16 @@ _GL_WARN_ON_USE (shutdown, "shutdown is not always POSIX 
compliant - "

 #if @GNULIB_ACCEPT4@
 /* Accept a connection on a socket, with specific opening flags.
-   The flags are a bitmask, possibly including O_CLOEXEC (defined in <fcntl.h>)
-   and O_TEXT, O_BINARY (defined in "binary-io.h").
+   The flags are a bitmask, possibly including SOCK_NONBLOCK,
+   SOCK_CLOEXEC, and O_TEXT, O_BINARY (defined in "binary-io.h").
    See also the Linux man page at
    <http://www.kernel.org/doc/man-pages/online/pages/man2/accept4.2.html>.  */
+# ifndef SOCK_CLOEXEC
+#  define SOCK_CLOEXEC O_CLOEXEC
+# endif
+# ifndef SOCK_NONBLOCK
+#  define SOCK_NONBLOCK O_NONBLOCK
+# endif
 # if @HAVE_ACCEPT4@
 #  if !(defined __cplusplus && defined GNULIB_NAMESPACE)
 #   define accept4 rpl_accept4