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

Socket refactors #3076

Merged
merged 2 commits into from
Aug 2, 2016
Merged

Conversation

ysbaddaden
Copy link
Contributor

I extracted the Socket documentation and code harmonization from #2829 along with improvements to UNIXServer and UNIXSocket so they are on par with TCPServer and TCPSocket, and a few more specs. I also fixed UNIXServer deleting the existing socket before binding the socket from #3068.

Harmonize coding styles, variables naming, and methods called across
the different classes. Write missing documentation.

Add missing `open` (with arguments) and `accept?` methods to
UNIXServer so it's on par with TCPServer.

Disable ability to select a protocol for `UDPSocket.pair` since the
general methods (`new` and `open`) don't allow it, and OSes don't
support any other protocol than the default `IPPROTO_IP` (0).

Avoid to leak a file descriptor when creating a UNIXSocket or
UNIXServer when the path is too long than supported by the OS. Also
raises ArgumentError instead of Exception.
UNIXServer shouldn't remove the existing socket path if it already
exists, but let the OS report an error and raise, instead of
hijacking existing servers listening on the socket. This allows
allows to use abstract socket addresses (containing null bytes).

fixes crystal-lang#3068
@asterite
Copy link
Member

asterite commented Aug 2, 2016

@ysbaddaden Thank you so much for this! Docs + cleaned up code = win! :-)

@asterite asterite merged commit 68f39a0 into crystal-lang:master Aug 2, 2016
@ysbaddaden ysbaddaden deleted the std-socket-refactors branch August 2, 2016 14:47
@asterite asterite added this to the 0.19.0 milestone Aug 2, 2016
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.

None yet

3 participants