-
-
Notifications
You must be signed in to change notification settings - Fork 447
Convert ZydisDecoder field decoder_mode to a bitmap. #435
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
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I like this change, but it seems like some stuff needs to be fixed before merging 🙂
The regression tests are failing and the fuzzer complains as well.
Make decoder_mode a bitmap instead of an array of booleans for space efficiency.
2e27d4b
to
261f0ee
Compare
Hi, Thank you for having a look. It looks like the |
@jpidancet Thanks for fixing! I will merge if @athre0z agrees as well. @athre0z @th0rex fyi, this is a breaking change for the JS, Rust, etc. bindings 🙂 For v5 I would like to change the public API for this entirely ( |
We can do this in a non-breaking manner by just adding such a function without removing the old one. I think this would be preferable here since maintaining the old function is very little effort. We can mark the old function as deprecated and remove it in a few years, or just keep it around forever. Changing the constants is not API breaking, only ABI breaking, so can be done in a minor version. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, good change!
Make decoder_mode a bitmap instead of an array of booleans for space efficiency.