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

ESMPy: Fix type cast to support large Arrays #267

Merged
merged 1 commit into from
Jul 31, 2024

Conversation

billsacks
Copy link
Member

In a few places, the size of an Array was being cast to a c_int. This caused an overflow if the size exceeded 32 bits. An example of the issue is reported here, and I have reproduced this:
https://github.com/orgs/esmf-org/discussions/263

From some reading, I think the correct cast here should be to ssize_t, which resolves to a long int (64 bits) on a 64-bit architecture. I am not 100% positive of this, but this seems consistent with what I have found; e.g., see:

In a few places, the size of an Array was being cast to a c_int. This
caused an overflow if the size exceeded 32 bits. An example of the issue
is reported here, and I have reproduced this:
https://github.com/orgs/esmf-org/discussions/263

From some reading, I *think* the correct cast here should be to ssize_t,
which resolves to a long int (64 bits) on a 64-bit architecture. I am
not 100% positive of this, but this seems consistent with what I have
found; e.g., see:
- Argument list in the PyMemoryView_FromMemory function:
  https://docs.python.org/3/c-api/memoryview.html
- https://docs.python.org/3/c-api/intro.html#c.Py_ssize_t
- https://stackoverflow.com/questions/20987390/cython-why-when-is-it-preferable-to-use-py-ssize-t-for-indexing
@billsacks billsacks requested a review from rokuingh July 23, 2024 22:23
@billsacks billsacks changed the title Fix type cast to support large Arrays ESMPy: Fix type cast to support large Arrays Jul 23, 2024
@billsacks billsacks self-assigned this Jul 23, 2024
@billsacks
Copy link
Member Author

ESMPy testing passes with this change on my Mac.

@billsacks
Copy link
Member Author

@rokuingh - I'm going ahead and merging this, but if you want to look at this later and notice any issues, I can change it.

@billsacks billsacks merged commit 75cee0a into develop Jul 31, 2024
2 checks passed
@billsacks billsacks deleted the esmpy_support_large_arrays branch July 31, 2024 15:31
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.

1 participant