Skip to content

Adds Buf impl for VecDeque<impl Buf>#822

Open
b01o wants to merge 10 commits intotokio-rs:masterfrom
b01o:vecdeque-bytes
Open

Adds Buf impl for VecDeque<impl Buf>#822
b01o wants to merge 10 commits intotokio-rs:masterfrom
b01o:vecdeque-bytes

Conversation

@b01o
Copy link
Copy Markdown

@b01o b01o commented Mar 5, 2026

This PR adds an implementation of the Buf trait for VecDeque<impl Buf>.

Sometimes we need to store multiple non-contiguous Bytes at runtime. Using Chain can be limiting because it doesn't provide a concrete type for a collection of buffers. Implementing Buf for VecDeque<impl Buf> gives a handy way to view these disjoint chunks as a single logical buffer.

Update:

  • Buf is implemented for VecDeque<impl Buf> instead of VecDeque<Bytes>.
  • implements chunks_vectored for VecDeque<impl Buf>
  • only return empty chunks if remaining() is 0
  • adds a test for chunks_vectored()

Co-authored-by: Alice Ryhl <aliceryhl@google.com>
@b01o b01o changed the title Adds Buf impl for VecDeque<Bytes> Adds Buf impl for VecDeque<impl Buf> Mar 5, 2026
change `let .. else ..` to `expect`
Comment on lines +47 to +53
fn chunk(&self) -> &[u8] {
if let Some(b) = self.front() {
b.chunk()
} else {
&[]
}
}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Now that I think of it more, this violates the implementer notes on chunk(). Since we accept any VecDeque, there could be empty elements within it, something that common BufList implementations prevent 1 2

chunk() should return an empty slice if and only if remaining() returns 0

Footnotes

  1. https://docs.rs/crate/hyper/1.8.1/source/src/common/buf.rs#17-21

  2. https://docs.rs/crate/watermelon-proto/0.1.8/source/src/util/buf_list.rs#22-27

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

I see what you mean, let me try to make it follow the contract.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

If we need a bugfix to existing code, it should be a separate PR.

Co-authored-by: Paolo Barbolini <paolo@paolo565.org>
Comment on lines +57 to +61
for buf in self {
if n >= dst.len() {
break;
}
n += buf.chunks_vectored(&mut dst[n..]);
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This loop must exit if the chunks written by buf have a combined length smaller than buf.remaining(), because you must read everything from the first buffer before you start reading from the second buffer. Please add a test that would have caught this.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

The access issue should be fixed now. Let me know if I missed anything.

b01o added 2 commits March 5, 2026 18:52
This prevents subsequent buffers from being incorrectly processed before the current buffer's remaining data.
Comment on lines +92 to +93
#[cfg(test)]
mod tests {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Tests go in tests/ directory outside of src/ unless they have to be here.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Now under tests/test_buf.rs.

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.

3 participants