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

Another integer overflow issue #138

Closed
lkeegan opened this issue Jan 29, 2025 · 0 comments · Fixed by #139
Closed

Another integer overflow issue #138

lkeegan opened this issue Jan 29, 2025 · 0 comments · Fixed by #139

Comments

@lkeegan
Copy link
Contributor

lkeegan commented Jan 29, 2025

When using a 32-bit integer for indexing, the data array in the C code has size n_points * n_dim and is indexed using a 32bit int type (see e.g. the PA macro).

Currently the condition to use a 32-bit int is that n_points < UINT_MAX, but then the above can overflow.

The correct condition should instead be n_points * n_dim < UINT_MAX

lkeegan added a commit to lkeegan/pykdtree that referenced this issue Jan 29, 2025
- correct the condition for using 32bit int indices
  - previous condition: `n_points < UINT_MAX`
  - however the data array in the C code has size `n_points * n_dim` and is indexed using a 32bit int type (see e.g. the PA macro)
  - this means results were incorrect for the case where `n_points < UINT_MAX` and `n_points * n_dim >= UINT_MAX`
  - new condition: `n_points * n_dim < UINT_MAX`
  - resolves storpipfugl#138
- render mako template
  - the generated _kdtree_core.c did not contain latest changes from _kdtree_core.c.mako (sorry my mistake)
  - this meant results were incorrect for the case `n_points < UINT_MAX` and `n_query * k >= UINT_MAX`
  - resolves storpipfugl#137
- use signed 64bit ints for the OpenMP loop on all platforms
  - to support OpenMP versions < 3 which only allow signed integers as loop counters
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 a pull request may close this issue.

1 participant