-
Notifications
You must be signed in to change notification settings - Fork 616
ida-explorer: replace deprecated IDA API find_binary with bin_search #2011
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
ida-explorer: replace deprecated IDA API find_binary with bin_search #2011
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.
Thank you @s-ff ! This looks great so far, I’ve left comments for you to address. No need for a new PR - push changes to this existing PR (CI runs with each push). Also, please locally execute and post here the results of running our IDA test script. Let us know here if you have any questions about running our IDA test script.
Hi @mike-hunhoff, I have run the test_ida_features.py test script using the following steps:
Here is the output of the test:
|
patterns = ida_bytes.compiled_binpat_vec_t() | ||
encoding = ida_nalt.get_default_encoding_idx(ida_nalt.BPU_1B) |
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.
find_byte_sequence
may be called many times. Can we move the initialization of patterns
and encoding
to the beginning of the file at the global scope so they are only called once? If so, while making this change let's update their names to be more descriptive, e.g. IDA_BYTES_PATTERNS
and IDA_NALT_ENCODING
.
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.
As requested, I have declared the global variables for reuse.
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.
@s-ff please confirm that both of these can be reused without needing to reinitialize before each use.
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.
Please confirm that IDA_BYTES_PATTERNS
can be reused as it appears that ida_bytes.parse_bin_pat_str
modifies it when called.
Hi @mike-hunhoff, Good point - Here is a snippet demonstrating this case: Here is the output of the test:
Please let me know if you need anything else before you merge this. |
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 🚀
This change closes #1606 by replacing the deprecated IDA API
find_binary
withbin_search
.Checklist