Skip to content

Conversation

tmiasko
Copy link
Contributor

@tmiasko tmiasko commented Jul 6, 2020

Previously, when non-empty sequence of empty IoSlices have been provided
to write_all_vectored, the buffers would be written as is with
write_vectored, and subsequently the return value of Ok(0) would be
misinterpreted as an error.

Avoid writes without any data by advancing the buffers first. This is
analogous to behaviour of write_all which makes no attempt to write if
there is no data.

Includes #2181.

Thomasdezeeuw and others added 2 commits June 19, 2020 17:40
In Cargo.toml the feature is called "write-all-vectored" (with "-"), in
the code it used "write_all_vectored" (with "_"), this commit fixes the
code to use the correct name.

Also fixes the example.
Previously, when non-empty sequence of empty IoSlices have been provided
to write_all_vectored, the buffers would be written as is with
write_vectored, and subsequently the return value of Ok(0) would be
misinterpreted as an error.

Avoid writes without any data by advancing the buffers first. This is
analogous to behaviour of write_all which makes no attempt to write if
there is no data.
Copy link
Contributor

@Thomasdezeeuw Thomasdezeeuw left a comment

Choose a reason for hiding this comment

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

LGTM, @taiki-e could take a look at this or #2181?

@Thomasdezeeuw
Copy link
Contributor

Ping @taiki-e.

@Thomasdezeeuw
Copy link
Contributor

rust-lang/rust#74088 is merged, can this be merged as well?

@Thomasdezeeuw
Copy link
Contributor

Since this has been reviewed, can it also be merged?

@seanmonstar seanmonstar merged commit 79679b3 into rust-lang:master Aug 9, 2020
@tmiasko tmiasko deleted the write-all-vectored-empty branch August 9, 2020 15:30
@taiki-e taiki-e added the A-io Area: futures::io label Jan 11, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-io Area: futures::io
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants