-
Notifications
You must be signed in to change notification settings - Fork 3
Improve byte array serialization #12
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
Conversation
It's a bit of a pain that dtolnay completely ignores my pr to serde_bytes. Maybe we should publish a fork of serde-bytes as |
src/util.rs
Outdated
/// the constant size `N`. | ||
#[derive(Debug, Deserialize, Serialize)] | ||
#[serde(transparent)] | ||
pub struct BytesArray<const N: usize>(Bytes<N>); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would rather take the approach of wrapping a [u8;N]
and manually implementing Serialize
, Deserialize
and Deref
on them (which I think also gives us the ConstantTimeEq
implementation. This avoids the unnecessary overhead of the Vec
and the runtime checks.
Maybe just take my existing implementation?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That was my first approach, but I’d rather not manually implement Deserialize
here. That just duplicates code from serde-bytes
or heapless-bytes
and requires more testing than just wrapping Bytes
.
Does this have to be a fork? Can’t we have a |
It doesn't necessarily have to be a fork no. We could have just the |
I’d prefer a separate crate. Mabye other people are also interested in using this specialization without pulling in |
I pushed one at https://github.com/Nitrokey/serde-byte-array |
404 – is it a private repository? |
I made it public |
Thanks! I’ll make a PR with some minor changes and then we can release. |
Per default, byte arrays are serialized inefficiently by serde. serde-byte-array provides a wrapper type with a more efficient serialization format. Fixes trussed-dev#11
44283fe
to
84fac10
Compare
Per default, byte arrays are serialized inefficiently by serde.
serde_bytes provides workarounds for byte slice serialization but does not support arrays yet. heapless_bytes improves the serialization for arrays but does not guarantee their length. Therefore we introduce a helper type, BytesArray, that uses heapless_bytes’ serialization but enforces a constant length.serde-byte-array provides a wrapper type with a more efficient serialization format.
Fixes #11