Skip to content

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