Soundness Fix for the Trigger struct as well as added unsafe call for the QuickTune struct since the type is not yet fully represented with rusts type system, and the fact the C struct is a union that we do not enforce.#48
Conversation
… the QuickTune struct since the type is not yet fully represented with rusts type system, and the fact the C struct is a union that we do not enforce.
| /// Precautionary test since the From<Backend> impl has 2 casts as I believe the [bladerf_backend] type is not consistent across platforms. | ||
| fn backend_enum_conversion_test() { |
There was a problem hiding this comment.
Can you elaborate on this? What kind of failure mode are you trying to guard against?
The variants in the header file have ordinals 0..=100 so we shouldnt have any issues with representing that in our #[repr(i32)] enum Backend { ... } (even if enum bladerf_backend is sized differently on different platforms)
There was a problem hiding this comment.
I put this in primarily as a sanity check if there are future code changes. I don't think it should be needed, but because of the double as cast, I was thinking about going through and adding some sort of brute force tests to anything that is repr(C). I did not really put a lot of though into this specific item, it was more of an addition so I would not have to think
|
I have double checked and this change does work |
TroyNeubauer
left a comment
TroyNeubauer
left a comment
There was a problem hiding this comment.
Makes sense. Ill look into fixing CI
Ok, thank you! |
3 participants
The trigger struct wav converted into the C ABI representation as 2 as casts to different pointer types. The direct conversion was invalid as the Rust struct was not repr C.
While the QuickTune struct is Repr C the C repr is actually a union. So this must be accounted for. The functions that utilize QuickTine have been marked as unsafe for now.