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

Drop Crystal::System::Socket#system_send #14637

Merged

Conversation

straight-shoota
Copy link
Member

@straight-shoota straight-shoota commented May 28, 2024

The intention of this method is equivalent to#system_write and the implementations are also identical.
There's no need for this duplication.

This does not affect the public API method Socket#send though. It's not identical to Socket#write because the latter is buffered and it ensures the entire data is sent.

Ref: crystal-lang/rfcs#7 (comment)

The intention of this method is equivalent to`#system_write` and the
implementations are also identical.
There's no need for this duplication.
@beta-ziliani beta-ziliani added this to the 1.13.0 milestone May 28, 2024
@ysbaddaden
Copy link
Contributor

ysbaddaden commented May 28, 2024

POSIX says (not just the Linux man page):

If the socket argument refers to a socket and the flags argument is 0, the send() function is equivalent to write().
https://pubs.opengroup.org/onlinepubs/9699919799/functions/send.html

So there really isn't a difference between the implementations in Crystal since we call send() without flags.

@ysbaddaden
Copy link
Contributor

ysbaddaden commented May 28, 2024

I can't find much information about Windows' WSASend() function, though. We use the WSASend() function to implement both #system_send and #system_write and both methods are just plain identical.

@straight-shoota straight-shoota merged commit 28fd0dc into crystal-lang:master May 29, 2024
61 checks passed
@straight-shoota straight-shoota deleted the refactor/drop-system_send branch May 29, 2024 13:30
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