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

support disassembling bytes from memoryview #1773

Merged
merged 3 commits into from
Nov 13, 2021

Conversation

huettenhain
Copy link
Contributor

Recreation of #1758 relative to the next branch.

@kabeor
Copy link
Member

kabeor commented Nov 9, 2021

could you provide a testcase for this?

@kabeor
Copy link
Member

kabeor commented Nov 10, 2021

@huettenhain I tried to verify this PR with following script

(a part of test_basic.py)

            md = Cs(arch, mode)
            code = memoryview(code)  # added this to check if memoryview work

            if syntax is not None:
                md.syntax = syntax

            for insn in md.disasm(code, 0x1000):
                print("0x%x:\t%s\t%s" % (insn.address, insn.mnemonic, insn.op_str))

but get this issue

File "capstone\bindings\python\capstone\__init__.py", line 1103, in disasm
    code = ctypes.byref(ctypes.c_char.from_buffer(code))
TypeError: underlying buffer is not writable

I think you should fix the compatibility of memoryview and ctypes

@huettenhain
Copy link
Contributor Author

Hey! Sorry for the delay. Indeed I was not aware that ctypes would require the buffer to be writable. I think it might be best to simply include this as a check, I'll update the PR soon.

@huettenhain
Copy link
Contributor Author

Hey @kabeor I didn't manage to build this locally, otherwise I would have tested it myself. I updated the PR so that it should work on any writable buffer type, including bytearray and memoryview.

@kabeor
Copy link
Member

kabeor commented Nov 13, 2021

@huettenhain I tried to write a test case to verify your PR, but it didn't work. Can you write a test case function in bindings/python/test_basic.py?

@huettenhain
Copy link
Contributor Author

I added a test and fixed one remaining bug in the code.

@kabeor
Copy link
Member

kabeor commented Nov 13, 2021

Test confirmed. Thanks for your contribution.

@kabeor kabeor merged commit 1d1f5e8 into capstone-engine:next Nov 13, 2021
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