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

Change protocol socket fd to Socket::Handle #10772

Merged

Conversation

straight-shoota
Copy link
Member

@straight-shoota straight-shoota commented May 31, 2021

This fixes a couple of refactors missed in #10706. Socket uses Handle as a platform-independent handle type. The protocol sockets should do the same.
On Linux, Handle is Int32, so this change has no real effect, but it enables those methods to work on windows.

@straight-shoota straight-shoota added kind:refactor platform:windows Windows support based on the MSVC toolchain topic:stdlib:networking labels May 31, 2021
@HertzDevil
Copy link
Contributor

Wouldn't this expose an internal type name to a public API?

I'm asking because I remembered a similar issue with File, but couldn't find it here.

@straight-shoota
Copy link
Member Author

I don't think Handle should be supposed to be considered internal. It's an abstraction to platform-specific socket handlers. As long as the API exposes methods that return or receive such handlers, that type is exposed as well. Well technically, it's an alias and thus transparently resolves to the platform-specific type at compile time. But it serves as the generic, portable type for these handlers.

@yxhuvud
Copy link
Contributor

yxhuvud commented Jun 2, 2021

In practice, the underlying handles will be needed to interact with external libraries, so they can't really be totally private.

Copy link
Member

@beta-ziliani beta-ziliani left a comment

Choose a reason for hiding this comment

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

Sounds like a reasonable abstraction. I didn't check that it's consistently applied.

@straight-shoota straight-shoota added this to the 1.1.0 milestone Jun 2, 2021
@straight-shoota straight-shoota merged commit bf809ac into crystal-lang:master Jun 4, 2021
@straight-shoota straight-shoota deleted the fix/socket-handle branch June 4, 2021 08:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind:refactor platform:windows Windows support based on the MSVC toolchain topic:stdlib:networking
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants