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

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(a)redhat.com&gt; > > Signed-off-by: Martin Kletzander <mkletzan(a)redhat.com&gt;

> --- > 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