-
Notifications
You must be signed in to change notification settings - Fork 54
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
Allow users to specify a custom ghidrathon.save path via an environment variable #99
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.
Thanks @s-ff , nice work! I've left comments for your review. Please let me know if you have any questions.
@s-ff looks like lints are failing, please see https://github.com/mandiant/Ghidrathon/blob/main/doc/contributing.md for more information on running lints locally. |
Co-authored-by: Mike Hunhoff <mike.hunhoff@gmail.com>
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.
Great updates, I've left comments for your review
To make sure that Ghidrathon plugin aligns with configure_ghidrathon.py we need to make sure that the GHIDRATHON_SAVE_PATH is never set to "".
The following commit fixes incorrect login in the previous commit. Please refer to the explination below: if GHIDRATHON_SAVE_PATH is set to any value (other than None or ""), check if it exists and is a directory. elif GHIDRATHON_SAVE_PATH is an empty string, report an invalid directory to avoid confusion. else GHIDRATHON_SAVE_PATH is not, use default install path.
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.
Thanks @s-ff , we're almost ready merge. I've left two comments for you to address.
src/main/java/ghidrathon/interpreter/GhidrathonInterpreter.java
Outdated
Show resolved
Hide resolved
Added log messages to indicate whether the file path was read from the environment variable or the default path was used.
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.
Thanks @s-ff , LGTM 🚀
This PR closes #98.