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

Ensure non-empty buffers for large vectored I/O #138879

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

thaliaarchi
Copy link
Contributor

readv and writev are constrained by a platform-specific upper bound on the number of buffers which can be passed. Currently, read_vectored and write_vectored implementations simply truncate to this limit when larger. However, when the only non-empty buffers are at indices above this limit, they will erroneously return Ok(0).

Instead, slice the buffers starting at the first non-empty buffer. This trades a conditional move for a branch, so it's barely a penalty in the common case.

The new method limit_slices on IoSlice and IoSliceMut may be generally useful to users like advance_slices is, but I have left it as pub(crate) for now.

@rustbot
Copy link
Collaborator

rustbot commented Mar 24, 2025

r? @jhpratt

rustbot has assigned @jhpratt.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

@rustbot rustbot added O-hermit Operating System: Hermit O-unix Operating system: Unix-like S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-libs Relevant to the library team, which will review and decide on the PR/issue. labels Mar 24, 2025
@thaliaarchi
Copy link
Contributor Author

thaliaarchi commented Mar 24, 2025

Since this involves what we guarantee for these methods, I want to make sure T-libs-api sees it too:

r? libs-api

The original question that led to this patch was whether these APIs are guaranteed to return Ok(0) iff the buffers are all empty or EOF is reached. As it stands, the implementation breaks that guarantee. However, it is stated on read and write, but not the vectored variants. Users do not have access to max_iov(), so they cannot do this logic themselves and it would make no sense to push the responsibility to them.

@rustbot rustbot added the T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. label Mar 24, 2025
@rustbot rustbot assigned BurntSushi and unassigned jhpratt Mar 24, 2025
`readv` and `writev` are constrained by a platform-specific upper bound
on the number of buffers which can be passed. Currently, `read_vectored`
and `write_vectored` implementations simply truncate to this limit when
larger. However, when the only non-empty buffers are at indices above
this limit, they will erroneously return `Ok(0)`.

Instead, slice the buffers starting at the first non-empty buffer. This
trades a conditional move for a branch, so it's barely a penalty in the
common case.

The new method `limit_slices` on `IoSlice` and `IoSliceMut` may be
generally useful to users like `advance_slices` is, but I have left it
as `pub(crate)` for now.
@thaliaarchi thaliaarchi force-pushed the vectored-large-empty branch from 58e54e2 to d4bc926 Compare March 25, 2025 00:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
O-hermit Operating System: Hermit O-unix Operating system: Unix-like S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-libs Relevant to the library team, which will review and decide on the PR/issue. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants