Skip to content

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!

@HippoBaro HippoBaro force-pushed the poll_get_aligned_buffer branch from 102f0db to 4d91894 Compare October 27, 2021 13:56
@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!

@HippoBaro HippoBaro force-pushed the poll_get_aligned_buffer branch from 4d91894 to 12e67b5 Compare November 4, 2021 17:50
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 force-pushed the poll_get_aligned_buffer branch from 12e67b5 to 5850658 Compare November 4, 2021 18:17
@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