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

Add Postcard Codec #20

Merged
merged 3 commits into from
Jul 14, 2024
Merged

Add Postcard Codec #20

merged 3 commits into from
Jul 14, 2024

Conversation

Firaenix
Copy link
Contributor

No description provided.

let mut bytes = vec![];
reader.read_to_end(&mut bytes).map_err(DeserializationError::new)?;

let item = postcard::from_bytes(&bytes).map_err(DeserializationError::new)?;
Copy link
Member

@surban surban Jul 14, 2024

Choose a reason for hiding this comment

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

Would it be possible to use postcard::from_io here instead?

We want to avoid keeping the whole incoming data in a memory buffer, if possible.

Copy link
Contributor Author

@Firaenix Firaenix Jul 14, 2024

Choose a reason for hiding this comment

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

I tried that initially but it required us to pass in a sized slice that is big enough to fit all the data to be deserialised. I could have done something like

let mut bytes = [0; 2048];

postcard::from_io((reader, &mut bytes))...

But I figured that would cause more problems than it's worth.

https://docs.rs/postcard/latest/postcard/fn.from_io.html

Copy link
Member

Choose a reason for hiding this comment

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

A bit unfortunate, but probably the best we can do for now.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, it's kind of annoying. Maybe I can circle back if postcard makes any further amendments to their API to address this.

I'll raise an issue with them about it.

@surban surban merged commit c0ef98d into ENQT-GmbH:master Jul 14, 2024
24 checks passed
@surban
Copy link
Member

surban commented Jul 14, 2024

Thanks!

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.

2 participants