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

cython autoconvert uint8_t[32] using PyObject_FromCString() and it is obviously wrong #3234

Open
thefallentree opened this issue Nov 15, 2019 · 4 comments

Comments

@thefallentree
Copy link

@thefallentree thefallentree commented Nov 15, 2019

So I have

cdef struct A:
    uint8_t[32] buf

and when I access it , x = A.buf , default auto conversion is calling with PyObject_FromCString((const char*) A.buf) which rely on strlen() over an binary field, obviously doesn't work, the only way to get correct conversion is to use

 x = <bytes> A.buf[:32] 

which force cython to use PyObject_FromCStringWithSize() , which works. I'm using the latest 0.29.14 cython

@thefallentree

This comment has been minimized.

Copy link
Author

@thefallentree thefallentree commented Dec 10, 2019

ping ? anyone having the same issue?

@robertwb

This comment has been minimized.

Copy link
Contributor

@robertwb robertwb commented Dec 10, 2019

This is working as intended; char[N] is often used in C to store a zero-terminated string of up to 32 (including the terminator) characters. (Yes, C is ambiguous as this is also how to store a character array of exactly 32 characters. uint8_t is an alias for char.)

@thefallentree

This comment has been minimized.

Copy link
Author

@thefallentree thefallentree commented Dec 10, 2019

fixed length array should probably behave differently than char * , or in other terms, there should be some kinda of warning or strict mode to warn people anytime the NULL-term conversion is used. (because that could be wrong)

@robertwb

This comment has been minimized.

Copy link
Contributor

@robertwb robertwb commented Dec 11, 2019

The problem is that fixed length arrays of chars is ambiguous in C--it's often used for both null-terminated and non-null-terminated data (e.g. to allow it to be allocated as part of a struct). No matter which convention is picked, it's going to be wrong for many people, and changing it now would be backwards incompatible. At least writing buf[:32] (or even buf[:sizeof(buf)] gives an obvious way to resolve this ambiguity.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.