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

[Martin Kletzander]

On Tue, Jul 27, 2021 at 01:52:34PM +0100, Richard W.M. Jones wrote:
>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>

Also need to remove this leftover from squashing changes =)

>> ---
>>  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.
> */
>

OK, I'll use that.  I wanted to express why we need them and forgot to
write what they do.

>Also, can you wrap lines at ~72 chars.
>

Ah, I used 72 for mails and 80 for code, will change that.

>> +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, unfortunately there is a need for an extra (unrelated)
modification to finish this, but it is small and already prepared.  I'll
send it soon.

>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
>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/libguestfs/attachments/20210727/ea0fed89/attachment.sig>