Skip to content

OpenBSD compilation support (real fixes.)#950

Closed
douglas-carmichael wants to merge 1 commit intoactor-framework:masterfrom
douglas-carmichael:master
Closed

OpenBSD compilation support (real fixes.)#950
douglas-carmichael wants to merge 1 commit intoactor-framework:masterfrom
douglas-carmichael:master

Conversation

@douglas-carmichael
Copy link
Copy Markdown
Contributor

These are the fixes from my fork of the actor-framework repository to compile on OpenBSD.
(Tested on OpenBSD 6.6.)

Copy link
Copy Markdown
Member

@Neverlord Neverlord left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Welcome to CAF! It's always great to see a new contributor. 🎉

Aside from my comment regarding MSG_NOSIGNAL: are the CAF unit tests still passing when not setting AI_V4MAPPED?

// platform-dependent SIGPIPE setup
#if defined(CAF_MACOS) || defined(CAF_IOS) || defined(CAF_BSD)
// Use the socket option but no flags to recv/send on macOS/iOS/BSD.
// (OpenBSD doesn't have SO_NOSIGPIPE)
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This patch seems incorrect to me. It may get past the compiler, but I question whether it achieves the desired behavior of blocking SIGPIPE. Looking into the man pages, it looks like OpenBSD provides the same API as Linux (i.e., MSG_NOSIGNAL): https://man.openbsd.org/sendmsg.2.

I think the correct patch is to replace defined(CAF_BSD) with defined(__FreeBSD__) in line 97 and then mention OpenBSD along Linux/Android in the #else branch.

I have no experience on OpenBSD, though. Do I miss something related to MSG_NOSIGNAL on OpenBSD?

Neverlord added a commit that referenced this pull request Oct 31, 2019
@Neverlord Neverlord mentioned this pull request Oct 31, 2019
@Neverlord
Copy link
Copy Markdown
Member

@douglas-carmichael thanks again for you contribution! I merged this PR into topic/openbsd and opened #955 with additional changes.

@Neverlord Neverlord closed this Oct 31, 2019
Neverlord added a commit that referenced this pull request Nov 10, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants