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

[issues-1567] Add varint parsing #1568

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

ikrivosheev
Copy link

Add support:

  • varint_u64
  • varint_u128

This PR close: #1567.

@Xiretza
Copy link
Contributor

Xiretza commented Oct 28, 2022

Wouldn't this be better implemented as a separate crate, much like https://crates.io/crates/nom-leb128? nom itself really only provides the most fundamental parsers and combinators.

@ikrivosheev
Copy link
Author

ikrivosheev commented Oct 28, 2022

Wouldn't this be better implemented as a separate crate, much like https://crates.io/crates/nom-leb128? nom itself really only provides the most fundamental parsers and combinators.

This make sense but i think varint is one of the fundamental parser.

@Xiretza
Copy link
Contributor

Xiretza commented Oct 28, 2022

I think for this to go into nom proper, there would have to be parsers for every integer type at the very least; though I wouldn't put too much effort into it until @Geal confirms it'd be accepted.

@ikrivosheev ikrivosheev requested a review from Geal as a code owner January 5, 2023 21:34
loop {
let (i, byte) = le_u8(remainder)?;
result += (<R as From<u8>>::from(byte & 0x7F)) << ofs;
if byte < 0x80 {

Choose a reason for hiding this comment

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

I believe this should exit if enough bytes for the target data type have been collected. Otherwise the right shift or addition will overflow.

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 support varint
3 participants