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

Request: Add FromBytes::read_from_prefix_split and read_from_suffix_split #1051

Closed
Tracked by #671
smalis-msft opened this issue Mar 15, 2024 · 5 comments
Closed
Tracked by #671
Labels
customer-request Documents customer requests.

Comments

@smalis-msft
Copy link

It would be handy to have a FromBytes::read_from_(prefix/suffix)_split that returns a tuple of the T and the unused portion of the byte slice. Ref::new_from_prefix isn't quite the same in case the byte slice is unaligned, but we don't want to derive Unaligned on the type.

@smalis-msft smalis-msft added the customer-request Documents customer requests. label Mar 15, 2024
@jswrenn
Copy link
Collaborator

jswrenn commented Mar 15, 2024

Something like this (playground)?

fn read_from_prefix_split<T>(bytes: &[u8]) -> Option<(T, &[u8])>
where
    T: Sized + FromBytes,
{
    Ref::<_, Unalign<T>>::new_unaligned_from_prefix(bytes)
        .map(|(r, s)| (r.read().into_inner(), s))
}

@smalis-msft
Copy link
Author

smalis-msft commented Mar 15, 2024

Yeah that looks about right. I mean there's a number of ways to do it, you can just call FromBytes::read_from_prefix and then index into your byte buffer by sizeof<T>, but having it provided on FromBytes and able to be used in a small oneliner would be nice.

@jstarks
Copy link

jstarks commented Mar 19, 2024

In our code, we often parse structures with fixed headers and then some dynamic portion, from a potentially unaligned buffer. To support this, it has been nice to have both options readily available (both preserving the suffix bytes and not), without having to reach for Ref.

Ref is powerful, but it's certainly more tedious to use. And it's arguably more dangerous (from a correctness perspective, not a safety one): since reading from an unaligned buffer requires even more typing than reading from/aliasing an aligned one, developers get lazy and just assume their buffer is aligned. And in their testing, it is. Then something subtle changes in the code, possibly far away from the call site, and runtime checks start failing.

Or developers write something like let foo = Foo::read_from_prefix(buf)? followed by let buf = &buf[..size_of::<Foo>()], which is error prone from the type name duplication. (I guess size_of_val(&foo) would also fix this, but it's still tedious to write.)

To solve this, we have a private crate that provides these kind of extra methods for use with zerocopy, but we'd love to just standardize on a single crate. Since these methods aren't already in zerocopy, I'm curious if we're "holding it wrong"--maybe there's a better way to do this kind of parsing? Do most users have aligned buffers? Or have unaligned struct definitions?

@joshlf
Copy link
Member

joshlf commented Mar 27, 2024

@jstarks Is that private crate open source? It'd be great to see what else you're needing to work around so that we can support your use cases!

@joshlf
Copy link
Member

joshlf commented Apr 24, 2024

Closed in #1059

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
customer-request Documents customer requests.
Projects
None yet
Development

No branches or pull requests

4 participants