Draft backward incompatible: proposal to address UB FixedInt - #30
Conversation
|
Thank you, your changes clean up a bunch of old work-arounds back from (I checked blame) around Rust 1.13. It's definitely nicer like this. |
|
Hey - are you interested in getting this merged? I have one or another idea on how to improve this further, but I really like your proposed structure. |
| pub trait FixedInt: Sized + Copy { | ||
| const REQUIRED_SPACE: usize; | ||
| /// Returns how many bytes are required to represent the given type. | ||
| fn required_space() -> usize; |
There was a problem hiding this comment.
Why do you remove the required_space function/constant? I agree that it is slightly cludgey, but the intent is that it gives easy access to the size of an encoded integer when using the encode_fixed() and decode_fixed() methods (e.g. when incrementally parsing a packet). Or will we tell users to simply use mem::size_of?
There was a problem hiding this comment.
nevermind; mem::size_of probably takes care of this best.
| dst.clone_from_slice(&self.to_le_bytes()); | ||
| } | ||
|
|
||
| #[cfg(target_endian = "little")] |
There was a problem hiding this comment.
I believe it was a mistake to introduce the target-dependent methods here. The byte order of some encoded data doesn't need to match the target endianness after all. (This is a note to myself, nothing you need to fix here.)
2 participants
FixedInt::REQUIRED_SPACEFixedInt::Bytesfn required_spaceswitch_endiannessimplementation requiredforbid(unsafe_code)