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

IO#write: return early if slice is empty #6269

Conversation

asterite
Copy link
Member

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).

@@ -14,13 +14,14 @@ module Crystal::System::FileDescriptor

private def unbuffered_write(slice : Bytes)
loop do
return if slice.empty?
Copy link
Contributor

Choose a reason for hiding this comment

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

perhaps until slice.empty?

Copy link
Member Author

Choose a reason for hiding this comment

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

"fixed"

@asterite asterite force-pushed the feature/6268-openssl-socket-write-empty-slice branch from dbafeac to 46cec0e Compare June 26, 2018 10:37
wait_writable
return if slice.empty?

begin
Copy link
Contributor

Choose a reason for hiding this comment

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

Why extra begin ... end?

Copy link
Member Author

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.

Copy link
Contributor

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.

Copy link
Member Author

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

@RX14 RX14 added this to the 0.25.1 milestone Jun 26, 2018
@RX14 RX14 merged commit df5f46a into crystal-lang:master Jun 26, 2018
@asterite asterite deleted the feature/6268-openssl-socket-write-empty-slice branch June 28, 2018 12:37
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

4 participants