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 <[email protected]>

Signed-off-by: Martin Kletzander <[email protected]>

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

Attachment: signature.asc
Description: PGP signature

_______________________________________________
Libguestfs mailing list
[email protected]
https://listman.redhat.com/mailman/listinfo/libguestfs

Reply via email to