-
-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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
IO#write: return early if slice is empty #6269
IO#write: return early if slice is empty #6269
Conversation
@@ -14,13 +14,14 @@ module Crystal::System::FileDescriptor | |||
|
|||
private def unbuffered_write(slice : Bytes) | |||
loop do | |||
return if slice.empty? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
perhaps until slice.empty?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"fixed"
dbafeac
to
46cec0e
Compare
wait_writable | ||
return if slice.empty? | ||
|
||
begin |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why extra begin ... end
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Because the ensure
must not run if the slice is empty. It's pointless to do anything at all if the slice is empty.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, I meant that ensure
could be attached to the loop
instead, like it was before.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It was attached to the def
Fixes #6268
Writing an empty slice should do nothing, so this PR adds this check to return early in that case. Fixes that case for OpenSSL socket, but also for others (no tests included because it's a bit hard to test, but the change isn't that complex).