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

Encode to Vec<u8> #378

Merged
merged 3 commits into from Jul 6, 2021
Merged

Encode to Vec<u8> #378

merged 3 commits into from Jul 6, 2021

Conversation

rubdos
Copy link
Contributor

@rubdos rubdos commented Oct 26, 2020

Adds Message::encode_to_vec(&self) -> Vec<u8> and Message::encode_length_delimited_to_vec() -> Vec<u8> on std-enabled configurations.

This is one line shorter than writing

let mut buf = Vec::new();
msg.encode(&mut buf).unwrap();

and replaces it by

let buf = msg.encode_to_vec();

The main reason I do that is because it's confused me (and others) before when writing PROST code in combination with symmetric crypto code (where encrypt(&self, &mut [u8]) overwrites allocated bytes, instead of extending the buffer like PROST).

It also omits the extra capacity checking of encode (since Vec is pre-allocated because this patch uses Vec::with_capacity()).

Fixes #154 in spirit, if you agree that Into<Vec<u8>> for Message is a slightly worse idea (i.e., .into doesn't specify whether to have length encoding or not, so it might yield slightly more unreadable code).

EDIT: CI failure seems to be due to use of a new rustc feature? I don't think it's this patch.

@rubdos
Copy link
Contributor Author

rubdos commented Jan 29, 2021

Rebased on master, subtle nod nod to the owners ;-)

@Jasperav
Copy link

Jasperav commented Apr 16, 2021

I guess this fixes https://github.com/danburkert/prost/issues/461, what do you think @rubdos? Adding this change as an example in the readme.md will help newcomers to this library :)

@rubdos
Copy link
Contributor Author

rubdos commented Apr 16, 2021

I agree that it'd fix #461 if it's added to the README. I'd have hoped to have some feedback from the maintainers though by now :/

@Jasperav
Copy link

@danburkert please review

@rubdos
Copy link
Contributor Author

rubdos commented May 6, 2021

This is a relatively simple QoL patch; would any of the people that have commit rights please take a look?

Copy link
Member

@LucioFranco LucioFranco left a comment

Choose a reason for hiding this comment

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

sorry for the delay on this!

@LucioFranco LucioFranco merged commit d8cb390 into tokio-rs:master Jul 6, 2021
@rubdos rubdos deleted the encode-to-vec branch July 6, 2021 14:29
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.

Add an into vec implementation?
3 participants