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

Add a poll_read_buf shim to tokio-util to mimic now deprecated AsyncRead API #2972

Merged
merged 5 commits into from
Oct 19, 2020

Conversation

udoprog
Copy link
Contributor

@udoprog udoprog commented Oct 16, 2020

While working to migrate a project to 0.3.0 I found uses of the now deprecated poll_read_buf API that needed to be migrated. This turned out to be nontrivial for types implementing BufMut which does not immediately provide an API to access an underlying contiguous buffer like Chain.

This adds a shim for it which provides the old API to ease migrations to 0.3.0. The naming and shape of the API is preliminary and I'll happily move and rename it to whatever is appropriate if it warrants inclusion. At the very least I'm putting it here in the hope so that people who are currently migrating might find it. It might also be a good idea to provide it as a library function since the generic implementation involves a sprinkle of unsafe it would be a shame for people to have to replicate.

@Darksonn Darksonn added A-tokio-util Area: The tokio-util crate M-io Module: tokio/io labels Oct 16, 2020
@Darksonn
Copy link
Contributor

Darksonn commented Oct 16, 2020

Thanks! I would like to see a function with this signature too:

pub async fn read_buf<R, B>(
    read: &mut R,
    buf: &mut B,
) -> io::Result<usize>
where
    R: AsyncRead + Unpin,
    B: BufMut,
{

It can just internally call poll_read_buf.

EDIT: Added async to signature.

@udoprog
Copy link
Contributor Author

udoprog commented Oct 16, 2020

Thanks! I would like to see a function with this signature too:

pub fn read_buf<R, B>(
    read: &mut R,
    buf: &mut B,
) -> io::Result<usize>
where
    R: AsyncRead + Unpin,
    B: BufMut,
{

It can just internally call poll_read_buf.

So I'm guessing this should either take a Context argument or be async?

@udoprog
Copy link
Contributor Author

udoprog commented Oct 16, 2020

Note: For whatever reason cargo fmt is not working on Windows, so I have to run rustfmt over all modified source files like the CI test does it.

image

@Darksonn
Copy link
Contributor

Yeah, it should be async. The cargo fmt issue is well known and described in CONTRIBUTING.md, not specific to windows, and I think it has to do with the feature flags.

@udoprog
Copy link
Contributor Author

udoprog commented Oct 17, 2020

Added read_buf helper fn per suggestion.

Couldn't find any non-dev dependency poll_fn, so implemented the future by hand which was pretty straight forward since it's Unpin.

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.

I think this is more or less ready.

tokio-util/src/io/poll_read_buf.rs Outdated Show resolved Hide resolved
tokio-util/src/io/read_buf.rs Outdated Show resolved Hide resolved
Co-authored-by: Alice Ryhl <alice@ryhl.io>
@Darksonn Darksonn merged commit 8d17261 into tokio-rs:master Oct 19, 2020
@udoprog udoprog deleted the poll-read-buf-shim branch October 19, 2020 10:03
hawkw added a commit that referenced this pull request Dec 3, 2020
Added

- io: `poll_read_buf` util fn (#2972).
- io: `poll_write_buf` util fn with vectored write support (#3156).
hawkw added a commit that referenced this pull request Dec 3, 2020
Added

- io: `poll_read_buf` util fn (#2972).
- io: `poll_write_buf` util fn with vectored write support (#3156).
hawkw added a commit that referenced this pull request Dec 3, 2020
### Added

- io: `poll_read_buf` util fn (#2972).
- io: `poll_write_buf` util fn with vectored write support (#3156).

Signed-off-by: Eliza Weisman <eliza@buoyant.io>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-tokio-util Area: The tokio-util crate M-io Module: tokio/io
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants