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

Clarification around parsing int32 from files #30

Closed
software-opal opened this issue Sep 5, 2017 · 2 comments
Closed

Clarification around parsing int32 from files #30

software-opal opened this issue Sep 5, 2017 · 2 comments

Comments

@software-opal
Copy link

software-opal commented Sep 5, 2017

I'm reviewing your code for a personal project and on Line 109 of the decoder, you create a parser for int32.

It seems to indicate that the type can be shorter(from the pad=True parameter), however in the actual function it reads exactly 4 bytes(type_size) irrespective of the size passed in. This renders the pad option redundant.

My reading of the function implies line 53 should read new_offset = offset + size. Related to this it might also be wise to assert that if pad: assert size <= type_size.

@oschwald
Copy link
Member

oschwald commented Sep 5, 2017

I think you may be right. A pull request would be welcomed. It would be nice to have a unit test for this. Re: the assert, we should throw an InvalidDatabaseError rather than an AssertionError in this case.

@software-opal
Copy link
Author

I will put one together, and yeah I was intending for it to throw the InvalidDatabaseError, I just wanted to make the condition clear

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

2 participants