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

Fixed: memory watch uses new argparse syntax of hexdump cmd #684

Merged
merged 3 commits into from
Jul 28, 2021

Conversation

theguy147
Copy link
Collaborator

Fixed: memory watch uses new argparse syntax of hexdump cmd

Description/Motivation/Screenshots

see #681

memory watch now uses the new argparse syntax of hexdump. Also fixed related hexdump usage instructions and documentation and added test to check for exceptions with the memory commands.

How Has This Been Tested?

Architecture Yes/No Comments
x86-32 ✔️
x86-64 ✔️
ARM ✖️
AARCH64 ✖️
MIPS ✖️
POWERPC ✖️
SPARC ✖️
RISC-V ✖️
make tests ✔️ note: its make test and not make tests, the template should be fixed

Checklist

  • My PR was done against the dev branch, not master.
  • My code follows the code style of this project.
  • My change includes a change to the documentation, if required.
  • My change adds tests as appropriate.
  • I have read and agree to the CONTRIBUTING document.

Copy link
Collaborator

@Grazfather Grazfather left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good. Thanks for this. One request for slightly more robust test.

@@ -247,6 +247,36 @@ def test_cmd_hexdump(self):
self.assertNoException(res)
return

def test_cmd_memory_watch(self):
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would be nice if you wrote something to memory and then ensured that it showed up in the context.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

agreed. i will put it on my todo list ;)

@Grazfather Grazfather merged commit 880f8b8 into hugsy:dev Jul 28, 2021
@Grazfather
Copy link
Collaborator

🎉

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

Successfully merging this pull request may close these issues.

2 participants