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

Check if override built-in command #543

Merged
merged 8 commits into from
Oct 20, 2018
Merged

Check if override built-in command #543

merged 8 commits into from
Oct 20, 2018

Conversation

ZetaTwo
Copy link
Contributor

@ZetaTwo ZetaTwo commented Oct 14, 2018

This PR aims to start a discussion on #230
This code fetches current gdb command on start-up and stores them in a list in the Command class.

The question is what to do with this list. As an example, I simply print out a warning but this is not really desirable since it prints out warnings for intended behaviour and is not actionable for the end-user. Another way to go would be to have a whitelist of the currently overridden commands: up, down, search, pwd and start and abort/exit if you try to override any other.

Thoughts?

@disconnect3d
Copy link
Member

Hey, thanks for the PR!

I haven't yet looked into the code (will probably do it tomorrow) but I am up for a whitelist of commands that we accept to override. If something doesn't fill into the whitelist, I think we could just fail (i.e. raise an exception or even exit gdb?).

@disconnect3d disconnect3d self-requested a review October 14, 2018 15:30
@disconnect3d disconnect3d added the enhancement For enhancements to existing features label Oct 14, 2018
@ZetaTwo
Copy link
Contributor Author

ZetaTwo commented Oct 14, 2018

Great, I updated the PR with the described behaviour. Any attempts to override anything other than the currently overridden commands: up, down, search, pwd and start will raise an exception. This is done with a hard-coded whitelist in the command class. If you prefer a more sophisticated way of doing it, please let me know.

@ZetaTwo
Copy link
Contributor Author

ZetaTwo commented Oct 14, 2018

The TravisCI check fails due to unrelated whitespace issues fixed separately in #544

@ZetaTwo
Copy link
Contributor Author

ZetaTwo commented Oct 18, 2018

Uhm, I messed up with my git:ing, please hold.

@ZetaTwo ZetaTwo changed the base branch from dev to beta October 18, 2018 19:55
@ZetaTwo ZetaTwo changed the base branch from beta to dev October 18, 2018 19:56
@ZetaTwo
Copy link
Contributor Author

ZetaTwo commented Oct 18, 2018

Ok, this should address all the points discussed. The solution for saving the pagination value is not beautiful but I didn't find a better way.

@disconnect3d disconnect3d self-assigned this Oct 19, 2018
Co-Authored-By: ZetaTwo <calle.svensson@zeta-two.com>
@disconnect3d disconnect3d merged commit 631c932 into pwndbg:dev Oct 20, 2018
@disconnect3d
Copy link
Member

Thanks!

@ZetaTwo ZetaTwo deleted the cmd_override_check branch October 20, 2018 13:10
@ZetaTwo
Copy link
Contributor Author

ZetaTwo commented Oct 20, 2018

Thanks for the merge and for a great project!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement For enhancements to existing features
Development

Successfully merging this pull request may close these issues.

None yet

2 participants