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 variant to DmaStreamReader::get_buffer_aligned #449

Merged
merged 2 commits into from
Nov 4, 2021

Conversation

HippoBaro
Copy link
Member

@HippoBaro HippoBaro commented Oct 22, 2021

What does this PR do?

Related to #445 and #446

Since DmaStreamReader is a stream, we have to play well with other
streams and make our functions easy to use from a polling context.
get_buffer_aligned is an async function and, as such, is hard to use
from there (not impossible, just hard).

If we look at the AsyncRead trait, one may see that it defines only
poll functions. Async functions are then built on top in the
AsyncReadExt. This is done because creating a future from a poll
function is trivial, while the other way around is hard.

Therefore, we create a new function poll_get_buffer_aligned and we
reimplement get_buffer_aligned as a wrapper around it:

pub async fn get_buffer_aligned(&mut self, len: u64) -> Result<ReadResult> {
    poll_fn(|cx| self.poll_get_buffer_aligned(cx, len)).await
}

@github-actions
Copy link

Greetings @HippoBaro!

It looks like your PR added a new or changed an existing dependency, and CI has failed to validate your changes.
Some possible reasons this could happen:

  • One of the dependencies you added uses a restricted license. See deny.toml for a list of licenses we allow;
  • One of the dependencies you added has a known security vulnerability;
  • You added or updated a dependency and didn't update the LICENSE-3rdparty.csv file. To do so, run the following and commit the changes:
$ cargo install cargo-license
$ cargo license --all-features -a -j --no-deps -d | jq -r '(["Component","Origin","License","Copyright"]) as $cols | map(. as $row | ["name", "repository", "license", "authors"] | map($row[.])) as $rows | $cols, $rows[] | @csv' > LICENSE-3rdparty.csv

Thank you!

@github-actions
Copy link

Greetings @HippoBaro!

It looks like your PR added a new or changed an existing dependency, and CI has failed to validate your changes.
Some possible reasons this could happen:

  • One of the dependencies you added uses a restricted license. See deny.toml for a list of licenses we allow;
  • One of the dependencies you added has a known security vulnerability;
  • You added or updated a dependency and didn't update the LICENSE-3rdparty.csv file. To do so, run the following and commit the changes:
$ cargo install cargo-license
$ cargo license --all-features -a -j --no-deps -d | jq -r '(["Component","Origin","License","Copyright"]) as $cols | map(. as $row | ["name", "repository", "license", "authors"] | map($row[.])) as $rows | $cols, $rows[] | @csv' > LICENSE-3rdparty.csv

Thank you!

Since `DmaStreamReader` is a stream, we have to play well with other
streams and make our functions easy to use from a polling context.
`get_buffer_aligned` is an async function and, as such, is hard to use
from there (not impossible, just hard).

If we look at the `AsyncRead` trait, one may see that it defines only
poll functions. Async functions are then built on top in the
`AsyncReadExt.` This is done because creating a future from a poll
function is trivial, while the other way around is hard.

Therefore, we create a new function `poll_get_buffer_aligned` and we
reimplement `get_buffer_aligned` as a wrapper around it:

```rust
pub async fn get_buffer_aligned(&mut self, len: u64) -> Result<ReadResult> {
    poll_fn(|cx| self.poll_get_buffer_aligned(cx, len)).await
}
```
Function returning `std::task::Poll` enums are usually called `poll_*`
to tell them apart from their async variants.
@HippoBaro HippoBaro merged commit df22c38 into DataDog:master Nov 4, 2021
@HippoBaro HippoBaro deleted the poll_get_aligned_buffer branch November 4, 2021 22:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants