This repository was archived by the owner on Oct 25, 2020. It is now read-only.
chore(macOS): fix compile and running error for macOS#6
Open
sharpbai wants to merge 4 commits intord235:masterfrom
Open
chore(macOS): fix compile and running error for macOS#6sharpbai wants to merge 4 commits intord235:masterfrom
sharpbai wants to merge 4 commits intord235:masterfrom
Conversation
Owner
|
On Sun, Oct 14, 2018 at 12:29:11PM -0700, sharpbai wrote:
macOS do not support SCTP protocol and SOCK_CLOEXEC flag. So I have changed
SCTP to TCP protocol and set SOCK_CLOEXEC to 0 for macOS.
It is not SCTP as it is on AF_LOCAL (a.k.a. AF_UNIX).
The correct semantics needed for the communication channel with the slirp thread is
SOCK_SEQPACKET otherwise the readv instruction in line 485 of libslirp.c can read
several requests in a single read (if the buffer is large enough to fit all).
We need the "packet boundaries".
For what concerns SOCK_CLOEXEC, even if MacOS does not support it, the socketpair cannot
be inherited by forked processes. It is useless and potentially disrupting.
As a workaround fcntl(2) can be used to set FD_CLOEXEC by the request named F_SETFD.
It is just a workaround as in a multithreaded environment the creation
of the socketpair and the flags setting must be done atomically.
If you have thoroughly tested your patch for MacOS (it would mean that it works
because of the specific implementation of SOCK_STREAM) I can consider to add
a patch for MacOS only.
I have not any proprietary operating system so I cannot test your patch so
you'd have to swear and take any responsability for MacOS ;-)
In no case I'll change SOCK_SEQPACKET to SOCK_STREAM for GNU-Linux.
I'll add a fcntl to set FD_CLOEXEC for Macs, too.
Happy hacking
renzo
|
Author
|
I have changed SOCK_STREAM to SOCK_DGRAM to ensure packet boundary. The socket buffer have been changed in order to avoid packet loss for udp socketpairs. The SOCK_CLOEXEC flag have also been set using fcntl. |
Author
|
I forgot to change back SOCK_CLOEXEC value for macOS. It is correct now. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
macOS do not support SCTP protocol and SOCK_CLOEXEC flag. So I have changed SCTP to TCP protocol and set SOCK_CLOEXEC to 0 for macOS.