Support pinning of IPythonBuffer#1411
Conversation
|
Looks like a pretty clean solution. I like it.
I'm fine either way if the changes are small and doesn't make it hard to review this PR. |
slozier
left a comment
slozier
left a comment
There was a problem hiding this comment.
Generally looks good to me (other than the change in memoryview error messages).
|
|
||
| if (flags.HasFlag(BufferFlags.CContiguous) && !_isCContig) | ||
| throw PythonOps.BufferError("memoryview: underlying buffer is not C-contiguous"); | ||
| throw PythonOps.BufferError("memoryview: underlying buffer is not C contiguous"); |
There was a problem hiding this comment.
I thought Python used "C-contiguous" with a dash?
struct.unpack("H", memoryview(b"AA")[::-1])BufferError: memoryview: underlying buffer is not C-contiguous
There was a problem hiding this comment.
memoryview does, but ctypes doesn't.
c_short.from_buffer(memoryview(bytearray(b"AA"))[::-1])TypeError: underlying buffer is not C contiguous
Unfortunately, tests for ctypes check for this form. Will see if I can get it both ways.
There was a problem hiding this comment.
Took a peek at the CPython ctypes implementation and it seems like from_buffer is the odd one:
ctypes.c_int.from_buffer(memoryview(bytearray(b"aaaa"))[::-1])
TypeError: underlying buffer is not C contiguous
but from_buffer_copy gives the memoryview error:
ctypes.c_int.from_buffer_copy(memoryview(bytearray(b"aaaa"))[::-1])
BufferError: memoryview: underlying buffer is not C-contiguous
|
Thanks! |
2 participants
Resolves #1297 though more can be done. Currently the lifetime of
MemoryHolderis not managed efficiently when used from C#. It was like that before this change and it does not affect Python code, which relies on the finalizers and the GC, but for C# calls, it can be done more efficiently usingDispose(). I think there are more improvements/modernizations/cleanups possible here, and likely in the wholectypesmodule, but that is for another PR (probably several).I have locally other changes in
ctypesthat would address the remaining part from #1404 and I used them for testing, not sure should they go in the separate PR (since they address a separate issue) or here (since they validate the code changes).