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

Fix mismatched integer sizes in compressionmodule.c on 32-bit #786

Closed
wants to merge 1 commit into from

Conversation

mdboom
Copy link
Contributor

@mdboom mdboom commented Feb 15, 2013

On 32-bit plaftorms, including Travis, we get this compiler warning.

astropy/io/fits/src/compressionmodule.c: In function ‘compression_compress_hdu’:
astropy/io/fits/src/compressionmodule.c:829:9: warning: passing argument 3 of ‘(struct PyObject * (*)(struct PyTypeObject *, int,  npy_intp *, int,  npy_intp *, void *, int,  int,  struct PyObject *)
)*(PyArray_API + 372u)’ from incompatible pointer type [enabled by default]
astropy/io/fits/src/compressionmodule.c:829:9: note: expected ‘npy_intp *’ but argument is of type ‘long int *’
astropy/io/fits/src/compressionmodule.c: In function ‘compression_decompress_hdu’:
astropy/io/fits/src/compressionmodule.c:906:5: warning: passing argument 3 of ‘(struct PyObject * (*)(struct PyTypeObject *, int,  npy_intp *, int,  npy_intp *, void *, int,  int,  struct PyObject *)
)*(PyArray_API + 372u)’ from incompatible pointer type [enabled by default]
astropy/io/fits/src/compressionmodule.c:906:5: note: expected ‘npy_intp *’ but argument is of type ‘long int *’

Particular when passing the list of dimensions, this could result in the very wrong array size being created.

This uses npy_intp instead of long so the array sizes match on 32-bit and 64-bit.

This may or may not be the cause of the segfaults on Travis, but either way we should probably fix this.

@embray
Copy link
Member

embray commented Feb 15, 2013

Weird. I wonder if this is related to #632, which was also on 32-bit Ubuntu, but that I was never able to reproduce on my own...

@embray
Copy link
Member

embray commented Feb 15, 2013

Also this will need to be backported to pyfits.

@mdboom
Copy link
Contributor Author

mdboom commented Feb 15, 2013

This doesn't fix the Travis builds, but seems like a good idea nonetheless.

@eteq
Copy link
Member

eteq commented Feb 15, 2013

This should also be in 0.2, right?

@embray
Copy link
Member

embray commented Feb 18, 2013

I don't know that it needs to be...it's causing any problems that we know of.

…mpy. This ensures the values are the correct size on both 32-bit and 64-bit platforms.
@ghost ghost assigned embray Apr 2, 2013
@embray
Copy link
Member

embray commented Apr 2, 2013

Manually merged in 97d0197.

@embray embray closed this Apr 2, 2013
@embray
Copy link
Member

embray commented Apr 2, 2013

I wonder if this is what was causing #632...

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.

None yet

3 participants