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

Use correct integer types in Cython #1913

niboshi opened this issue Dec 26, 2018 · 2 comments · Fixed by #2455

Use correct integer types in Cython #1913

niboshi opened this issue Dec 26, 2018 · 2 comments · Fixed by #2455


Copy link

@niboshi niboshi commented Dec 26, 2018

Currently incorrect integer types are used in Cython implementation.
It's less portable and perhaps more importantly, confusing for developers.

For example:

  • Pointers should be intptr_t.
  • Memory sizes should be size_t.
  • Memory offsets should be ptrdiff_t.
Copy link

@leofang leofang commented Sep 9, 2019

Hi @ninoshi, when I was working on PRs that would touch the driver wrappers, I noticed that in driver.pxd/driver.pyx there are many types that should have been changed from size_t to intptr_t but were left untouched in #1952, which made me thought that using size_t was the deliberately chosen convention for drivers, but looking at this issue I doubt it's the case.

Specifically, I am referring to the driver functions involving the following types:

ctypedef void* Context 'CUcontext'
ctypedef void* Deviceptr 'CUdeviceptr'
ctypedef void* Event 'struct CUevent_st*'
ctypedef void* Function 'struct CUfunc_st*'
ctypedef void* Module 'struct CUmod_st*'
ctypedef void* Stream 'struct CUstream_st*'
ctypedef void* LinkState 'CUlinkState'

all of which are just pointers but were represented using size_t. Is this an oversight? If so I can make a PR to fix them.

Copy link
Member Author

@niboshi niboshi commented Sep 9, 2019

I didn't deliberately leave them. I think I simply overlooked, so please feel free to fix them!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

2 participants