-
-
Notifications
You must be signed in to change notification settings - Fork 757
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
add nopi command it patchs full instructions #959
Conversation
I'm not a fan of having two commands that do the same thing, especially when the code is this similar. Why not simply add an option (like |
Thanks for pointing that out. There must have a been a mix-up somewhere, because I thought
Can you update your PR to reflect that? |
So... nop -> patch $pc 1-full-instruction nop --ni 10 -> patch $pc 10-full-instruction nop addr -> patch addr 1-full-instruction nop addr --ni 10 -> patch addr 10-full-instructions do you like this way? @hugsy I updated this comment, now it makes more sense |
IMO, I dont like the "switch idea", have a nop & nopi commands its more fast and easy to use+remember :D (and also easy to mantain, the code+tests will be more simple and easy to follow). So, what do you want? tell me and I will do it :D |
My point is, there should not be two commands: I thought the current So yes, let's have The syntax can be something like:
For instance (to pile on your examples):
WDYT ? |
Yeah, gotcha |
done! @hugsy what do you think now? |
Description/Motivation/Screenshots
nopi command patchs full instructions, nop command usually breaks disasm
Also, the current way using nop command + calculating bytes by hand/commands when you want patch N instructions is a pain in the ass hehe
This command should be more useful & user friendly than nop command
Against which architecture was this tested ?
"Tested" indicates that the PR works and the unit test (see
docs/testing.md
) run passes without issue.Checklist
dev
branch, notmain
.