Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow creating sockets from raw file descriptors #6894

Merged
merged 1 commit into from
Jan 2, 2019

Conversation

valpackett
Copy link
Contributor

This is useful for socket activation, sandboxes/fd-passing, etc.

@ysbaddaden
Copy link
Contributor

Use case is valid, but CI is red.

@straight-shoota
Copy link
Member

Needs fix and maybe a spec for the described behaviour.

@straight-shoota
Copy link
Member

@myfreeweb Are you still working on this?

@valpackett
Copy link
Contributor Author

I don't understand why that test is failing..

@straight-shoota
Copy link
Member

straight-shoota commented Dec 31, 2018

@myfreeweb The added TCPServer#initialize(fd) shadows TCPServer.new(port) because both can be called with a single Int32 argument.

I'd suggest to make fd an required named argument: def initialize(*, fd : Int32 ... An instance can be created using TCPServer.new(fd: fd).

This should be changed uniquely in all fd constructors.

@straight-shoota
Copy link
Member

It's a shame that such a change breaks only one mostly unrelated SSL spec 😢

@valpackett valpackett force-pushed the sock-from-fd branch 2 times, most recently from 1dd3a5c to 061f5f9 Compare December 31, 2018 15:25
@valpackett
Copy link
Contributor Author

Passes now

@straight-shoota
Copy link
Member

Awesome 👍 Could you add a doc to the now public Socket#initialize as well?

This is useful for socket activation, sandboxes, etc.
@valpackett
Copy link
Contributor Author

done

Copy link
Member

@straight-shoota straight-shoota left a comment

Choose a reason for hiding this comment

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

👍

src/socket.cr Show resolved Hide resolved
@RX14 RX14 added this to the 0.27.1 milestone Jan 2, 2019
@RX14 RX14 merged commit 3f7e2c9 into crystal-lang:master Jan 2, 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.

6 participants