search

simonrw / rust-fitsio

FFI wrapper around cfitsio in Rust
Apache License 2.0
34 stars 11 forks source link

Unaligned Pointer Read #274

Open petesmc opened 7 months ago

petesmc commented 7 months ago

This test using a i8 results in an unaligned pointer read...

https://github.com/simonrw/rust-fitsio/blob/1972db698dac94440f11e201eccbebe95ec9dace/fitsio/src/headers.rs#L246

... due to 

&value as *const $t as *mut c_void,

... in the below.

https://github.com/simonrw/rust-fitsio/blob/1972db698dac94440f11e201eccbebe95ec9dace/fitsio/src/headers.rs#L102-L125

Which calls ffpkyj in the cfitsio library like so:

ffpkyj(fptr, keyname, (LONGLONG) *(short *) value, comm, status); // <--- pointer to i8 gets converted into pointer to short.

I believe to fix this, you need to convert i8 -> i16 given you are using TSHORT, OR map i8 to TBYTE instead.

simonrw commented 7 months ago

Hi, thanks for reporting this issue, and I'm glad you're using the project! You are absolutely right, the types do not match up for u8.

Do you have a minimal example which I can use to reproduce this issue, and make sure it doesn't happen again?

petesmc commented 7 months ago

Sorry nothing I can share at the moment. I detected it because I'm rewriting part of cfitsio in rust which in debug mode checks for misaligned pointer reads.

simonrw commented 7 months ago

Ok thanks. I've merged in a fix and am ok to not include a test for now. Can you use the latest main commit and test for me please?

simonrw commented 3 months ago

@petesmc have you had a chance to test my fix?

petesmc commented 2 months ago

Looks good for bytes, but think you need to update the u16/i16 as well to TUSHORT/TSHORT