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

doc: Clarify EOF condition for AsyncReadExt::read_buf #3850

Merged
merged 3 commits into from
Jun 9, 2021
Merged

doc: Clarify EOF condition for AsyncReadExt::read_buf #3850

merged 3 commits into from
Jun 9, 2021

Conversation

emesterhazy
Copy link
Contributor

Motivation

The current documentation for read_buf says:

If the supplied buffer is not empty and the function returns Ok(0) then the source has reached an “end-of-file” event.

This confused me since I took "empty" to mean that the buffer had no readable data in it (like a vec with len() == 0).
I think part of the confusion is due to buffers like bytes::BytesMut using "empty" to refer to buffers that have
no readable data (i.e. a newly created buffer that hasn't been written to yet).

After reading the source code and chatting with @Darksonn it's clear that Ok(0) is returned when there's no more
room in the buffer to write additional bytes. This is "empty" in the sense that an empty in the sense that an empty struct is,
but not in the sense that an empty vec is (which IMO is a closer mental model to a buffer).

Solution

I propose we change the above sentence to read:

If the remaining capacity of the supplied buffer is greater than 0 and the function returns Ok(0) then the source has reached an "end-of-file" event.

The use of "remaining capacity" here is consistent with the phrasing bytes::BytesMut uses.

Comment on lines 183 to 186
/// On a successful read, the number of read bytes is returned. If the
/// supplied buffer is not empty and the function returns `Ok(0)` then
/// the source has reached an "end-of-file" event.
/// remaining capacity of the supplied buffer is greater than 0 and
/// the function returns `Ok(0)` then the source has reached an
/// "end-of-file" event.
Copy link
Contributor

Choose a reason for hiding this comment

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

I think it would read even better if we made a list with the two cases like we do on AsyncReadExt::read.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

👍 I pulled the comment down from read, but changed the second point for read_buf because I think the concept of "remaining capacity" makes more sense for BufMut than length since a BufMut has the concept of initialized length and uninitialized remaining capacity.

Here's the change for that line:

- 2. The buffer specified was 0 bytes in length.
+ 2. The buffer specified had a remaining capacity of zero.

@Darksonn Darksonn added A-tokio Area: The main tokio crate M-io Module: tokio/io T-docs Topic: documentation labels Jun 9, 2021
/// supplied buffer is not empty and the function returns `Ok(0)` then
/// the source has reached an "end-of-file" event.
/// If the return value of this method is `Ok(n)`, then it must be
/// guaranteed that `0 <= n <= buf.len()`. A nonzero `n` value indicates
Copy link
Contributor

Choose a reason for hiding this comment

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

The BufMut trait has no len method.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah, you're right. I'll update.

Copy link
Contributor

@Darksonn Darksonn left a comment

Choose a reason for hiding this comment

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

Thanks!

@Darksonn Darksonn merged commit d16e506 into tokio-rs:master Jun 9, 2021
@emesterhazy emesterhazy deleted the etm/async_read_ext_docs branch June 9, 2021 18:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-tokio Area: The main tokio crate M-io Module: tokio/io T-docs Topic: documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants