Add serde #139
Add serde #139
Conversation
JonathanWoollett-Light
force-pushed
the
serde
branch
from
062f1f5 to
ade545b
Compare
|
Thanks for this implementation! I've actually tried once but found it a little complicated and went and did something else but we did kind of need it... neat! :) |
| S: serde::Serializer, | ||
| { | ||
| // Create a temporary Vec to store the valid elements | ||
| let mut elements = alloc::vec::Vec::with_capacity(CAP); |
There was a problem hiding this comment.
is this alloc really necessary?
There was a problem hiding this comment.
since it's basically thrown away
| let mut elements = alloc::vec::Vec::with_capacity(CAP); | ||
|
|
||
| // Handle the case where the buffer might be empty | ||
| if self.readptr != self.writeptr { |
There was a problem hiding this comment.
I think it'd be better to just use the iterator here
|
|
||
| // Serialize the elements along with the buffer metadata | ||
| let mut state = serializer.serialize_struct("ConstGenericRingBuffer", 3)?; | ||
| state.serialize_field("elements", &elements)?; |
There was a problem hiding this comment.
I'm not sure we need to serialize the read and writeptr tbh. Those aren't useful in a serialization and I'd be very scared deserializing them. Since soundness depends on the state of these pointers, deserializing malformed data could be unsound. Semantically, a serialized ringbufer is just a collection of elements, which when deserialized can just be pushed into a new ringbuffer, which might end up with different states for readptr/writeptr
| buf[idx] = MaybeUninit::new(element); | ||
| } | ||
|
|
||
| Ok(ConstGenericRingBuffer { |
There was a problem hiding this comment.
unsound, when data is malformed. if readptr is maliciously (or accidentally) modified it'd read uninitialized elements
| S: serde::Serializer, | ||
| { | ||
| // Similar to Vec's implementation, we only serialize the actual elements | ||
| // Create an iterator over the valid elements |
There was a problem hiding this comment.
there is an iterator over the valid elements, with self.iter()
I sort of just threw this together for something I was testing, I expect there are probably more issues than even the ones you mentioned, I don't know if I'll find the time to get this into proper shape any time soon but maybe this is a small step. |
Implements
serde::Serializeandserde::Deserialize.This follows up #133