-
Notifications
You must be signed in to change notification settings - Fork 304
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
Consider providing Rewind-style traits #330
Comments
An idea I've thought about for a couple hours is to have a separate trait, if there are buffer types that couldn't support this (do they exist?). trait Rewind {
fn position(&self) -> usize;
fn rewind(&mut self, pos: usize);
} |
A convenient way to implement this would be a transaction-style adapter that stores the position at which it was constructed and can be either aborted or committed. |
As an added note: peeking util fns would be useful to add here. |
Also, instead of "rewinding" it probably should be generalized as scannable or indexable? I imagine buffers backed by linked lists would not implement this trait, but a rope could (despite That said, a linked list backed buffer could implement a strategy where the current position is saved by cloning the "iterator". |
Now I wonder if the trait should be specifically for the case of a buf backed by a single slice. Then there could be helpers like “get_line” |
@seanmonstar points out that a An alternate strategy would be for struct SliceBuf<'a> {
bytes: &'a [u8],
pos: usize,
} This type could implement misc helpers such as "get_line", "peek", ... misc other types could have: fn as_slice_buf(&self) -> SliceBuf<'_> {
// ...
} thoughts? @sfackler @seanmonstar |
I've been struggling with this while trying to convert an MQTT decoder to take a I could use I'm guessing that The transaction is tempting too (it'd be great if |
The initial idea was to have `decode()` take a `Buf` and `encode()` take a `ButMut`, as they semantically should. It would have enabled our users to use other buffer backends. Switching `encode()` to take a BufMut was straightforward, that's done. Concerning `decode()`, `Buf` currently lacks facilities to rewind the read position (tokio-rs/bytes#330) and to read a specified number of bytes (tokio-rs/bytes#129), making it impossible to implement the desired API unless we assume that our user is using a continuous-memory backend like BytesMut (and even then, the implementation isn't really pretty). It is possible to have `decode()` take a `Bytes` (as it semantically should) instead of a `BytesMut`, but because users typically want to pass a `BytesMut` to `TcpStrem.read()` it would require a `.freeze()` before passing to `decode()`, which is cumbersome and prevents buffer reuse. With all that said, this commit does move things in the right direction, and if upstream fixes the mentioned bugs it'll be that much easyer to switch.
The initial idea was to have `decode()` take a `Buf` and `encode()` take a `ButMut`, as they semantically should. It would have enabled our users to use other buffer backends. Switching `encode()` to take a BufMut was straightforward, that's done. Concerning `decode()`, `Buf` currently lacks facilities to rewind the read position (tokio-rs/bytes#330) and to read a specified number of bytes (tokio-rs/bytes#129), making it impossible to implement the desired API unless we assume that our user is using a continuous-memory backend like BytesMut (and even then, the implementation isn't really pretty). It is possible to have `decode()` take a `Bytes` (as it semantically should) instead of a `BytesMut`, but because users typically want to pass a `BytesMut` to `TcpStrem.read()` it would require a `.freeze()` before passing to `decode()`, which is cumbersome and prevents buffer reuse. With all that said, this commit does move things in the right direction, and if upstream fixes the mentioned bugs it'll be that much easyer to switch.
The initial idea was to have `decode()` take a `Buf` and `encode()` take a `ButMut`, as they semantically should. It would have enabled our users to use other buffer backends. Switching `encode()` to take a BufMut was straightforward, that's done. This solves a gotcha about using `bytes::BytesMut` vs `bytes::bytes_mut::BytesMut`, and enables the use of other buffer types, like `Vec`. Concerning `decode()`, `Buf` currently lacks facilities to rewind the read position (tokio-rs/bytes#330) and to read a specified number of bytes (tokio-rs/bytes#129), making it impossible to implement the desired API unless we assume that our user is using a continuous-memory backend like BytesMut (and even then, the implementation isn't really pretty). It is possible to have `decode()` take a `Bytes` (as it semantically should) instead of a `BytesMut`, but because users typically want to pass a `BytesMut` to `TcpStrem.read()` it would require a `.freeze()` before passing to `decode()`, which is cumbersome and prevents buffer reuse. With all that said, this commit does move things in the right direction, and if upstream fixes the mentioned bugs it'll be that much easyer to switch.
The use case for me would be to rewind to populate a checksum field.
The data header contains a bunch of fields. The checksum is computed over the payload and part of the header, so it's the last field I'll set. |
Sometimes when encoding or decoding, it can be desirable to "rewind" a read or write operation. Some examples:
u32
that describes the "length" of the rest of message, and then seeing thatremaining()
does not have "length" bytes, a user may wish to "unread" the "length" and wait for more data.See also #299
cc @sfackler
The text was updated successfully, but these errors were encountered: