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

Added lots of decoder tests and fixed lots of bugs. #38

Merged
merged 2 commits into from
Aug 19, 2015

Conversation

haberman
Copy link
Member

No description provided.

@haberman
Copy link
Member Author

To @scwhittle. I'm not going to worry about the Coveralls "failure."

@@ -558,9 +571,9 @@ uint32_t Hash(const string& proto, const string* expected_output, size_t seam1,

void CheckBytesParsed(const upb::pb::Decoder& decoder, size_t ofs) {
// We could have parsed as many as 10 bytes fewer than what the decoder
// previously accepted, since we can buffer up to 10 partial bytes internally
// previously accepted, since we can buffer up to 12 partial bytes internally

Choose a reason for hiding this comment

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

Lots of different numbers 10, 12, 14
should they all be 14? Maybe with explanation on where 14 was calculated

Copy link
Member Author

Choose a reason for hiding this comment

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

Done.

@@ -257,15 +286,14 @@ size_t upb_pbdecoder_suspend(upb_pbdecoder *d) {
d->ptr = d->residual;
return 0;
} else {
size_t consumed;
size_t ret = d->size_param - (d->end - d->checkpoint);

Choose a reason for hiding this comment

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

size_t consumed

haberman added a commit that referenced this pull request Aug 19, 2015
Added lots of decoder tests and fixed lots of bugs.
@haberman haberman merged commit 77d45ed into protocolbuffers:master Aug 19, 2015
@haberman haberman deleted the decoderfix2 branch February 18, 2016 21:18
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