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

Fix many minor bugs in several scripts #98

Merged
merged 3 commits into from
Jan 29, 2024
Merged

Fix many minor bugs in several scripts #98

merged 3 commits into from
Jan 29, 2024

Conversation

hugsy
Copy link
Owner

@hugsy hugsy commented Jan 20, 2024

Description/Motivation/Screenshots

restored to a workable state - for 2024.01 :

  • remote
  • windbg
  • assemble
  • libc_function_args is completely broken

How Has This Been Tested ?

"Tested" indicates that the PR works and the unit test (i.e. make test) run passes without issue.

  • x86-32
  • x86-64
  • ARM
  • AARCH64
  • MIPS
  • POWERPC
  • SPARC
  • RISC-V

Checklist

  • My code follows the code style of this project.
  • My change includes a change to the documentation, if required.
  • If my change adds new code,
    adequate tests have been added.
  • 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.

Style Q, fine otherwise.

self.__original_disassembler = ctx.instruction_iterator
ctx.instruction_iterator = cs_disassemble
else:
# `use-capstone` set to False
if ctx.instruction_iterator == cs_disassemble and self.__original_disassembler:
if (
Copy link
Collaborator

Choose a reason for hiding this comment

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

Do you prefer this style (four lines) over either 1 line or 2 with parens or 2 with \?

Copy link
Owner Author

Choose a reason for hiding this comment

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

I almost forgot: I started using black for gef-extras (only for now), and those kind of output were the result. I'm not sure how I feel yet about it.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Ah yeah, that may be why I didn't like black (remember I said it was too opinonated and I didn't like the opinion).

I am 100% for auto-formatting everything! That will be nice and easy to check. I don't love black but I could live with it.

Comment on lines +192 to +194
for insn in cs_disassemble(
location, length, skip=length * self.repeat_count, **kwargs
):
Copy link
Collaborator

Choose a reason for hiding this comment

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

I don't think we need (nor do I like) the hanging parens. Wastes lines imo.

Suggested change
for insn in cs_disassemble(
location, length, skip=length * self.repeat_count, **kwargs
):
for insn in cs_disassemble(
location, length, skip=length * self.repeat_count, **kwargs):

@hugsy hugsy merged commit e4dda90 into main Jan 29, 2024
7 checks passed
@hugsy hugsy deleted the fix_many_minor_bugs branch January 29, 2024 00:18
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