[Libguestfs] [libnbd PATCH v2 2/2] macOS: Simple cloexec/nonblock fix

[Richard W.M. Jones]

On Tue, Jul 27, 2021 at 01:31:05PM +0200, Martin Kletzander wrote:
> This is the most trivial way to fix the issue with macOS not having SOCK_CLOEXEC
> and SOCK_NONBLOCK.  This is the only way to make it work on such platform(s)
> unless they are fixed.
> 
> Signed-off-by: Martin Kletzander <mkletzan at redhat.com>
> 
> Signed-off-by: Martin Kletzander <mkletzan at redhat.com>
> ---
>  lib/internal.h                               |  7 ++
>  generator/states-connect-socket-activation.c |  2 +-
>  generator/states-connect.c                   | 11 +--
>  lib/utils.c                                  | 79 ++++++++++++++++++++
>  fuzzing/libnbd-fuzz-wrapper.c                |  4 +
>  fuzzing/libnbd-libfuzzer-test.c              |  4 +
>  6 files changed, 101 insertions(+), 6 deletions(-)
> 
> diff --git a/lib/internal.h b/lib/internal.h
> index 01f9d8ab5fea..12938aaa0444 100644
> --- a/lib/internal.h
> +++ b/lib/internal.h
> @@ -467,4 +467,11 @@ extern char *nbd_internal_printable_buffer (const void *buf, size_t count);
>  extern char *nbd_internal_printable_string (const char *str);
>  extern char *nbd_internal_printable_string_list (char **list);
>  
> +/*
> + * These are wrappers over socket(2) and socketpair(2) that work on macOS where
> + * SOCK_NONBLOCK and SOCK_CLOEXEC are not available.

It's still a bit unclear.  Could we say:

/* These are wrappers around socket(2) and socketpair(2).  They
 * always set SOCK_CLOEXEC.  nbd_internal_socket can set SOCK_NONBLOCK
 * according to the nonblock parameter.
 */

Also, can you wrap lines at ~72 chars.

> +int nbd_internal_socket(int domain,
> +                        int type,
> +                        int protocol,
> +                        bool nonblock)
> +{
> +  int fd;
> +
> +  /* So far we do not know about any platform that has SOCK_CLOEXEC and lacks
> +   * SOCK_NONBLOCK at the same time.
> +   *
> +   * The workaround for missing SOCK_CLOEXEC introduces a race which cannot be
> +   * fixed until support for SOCK_CLOEXEC is added (or other fix is implemented).
> +   */

Line wrapping here.

> +int
> +nbd_internal_socketpair (int domain, int type, int protocol, int *fds)
> +{
> +  int ret;
> +
> +  /*
> +   * Same as with nbd_internal_socket() this workaround for missing SOCK_CLOEXEC
> +   * introduces a race which cannot be fixed until support for SOCK_CLOEXEC is
> +   * added (or other fix is implemented).
> +   */

And here.

It's all good otherwise: ACK the series with the changes above.

Thanks,

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
Fedora Windows cross-compiler. Compile Windows programs, test, and
build Windows installers. Over 100 libraries supported.
http://fedoraproject.org/wiki/MinGW