Transmuting enums is UB

[Pzixel]

Looking at code I found that you are performing UB conversion from primitive to enum

anne-key/src/protocol.rs

Lines 10 to 35 in ac02810

	#[repr(u8)]
	#[non_exhaustive]
	#[derive(Debug, Copy, Clone)]
	pub enum MsgType {
	Reserved = 0,
	Error = 1,
	System = 2,
	Ack = 3,
	Reboot = 4,
	Macro = 5,
	Ble = 6,
	Keyboard = 7,
	Keyup = 8,
	Led = 9,
	FwInfo = 10,
	FwUp = 11,
	CustomLed = 12,
	CustomKey = 13,
	}
	impl From<u8> for MsgType {
	#[inline]
	fn from(b: u8) -> Self {
	unsafe { transmute(b) }
	}
	}

#[repr(u8)]
#[non_exhaustive]
#[derive(Debug, Copy, Clone)]
pub enum MsgType {
    Reserved = 0,
    Error = 1,
    System = 2,
    Ack = 3,
    Reboot = 4,
    Macro = 5,
    Ble = 6,
    Keyboard = 7,
    Keyup = 8,
    Led = 9,
    FwInfo = 10,
    FwUp = 11,
    CustomLed = 12,
    CustomKey = 13,
}

impl From<u8> for MsgType {
    #[inline]
    fn from(b: u8) -> Self {
        unsafe { transmute(b) }
    }
}

You may argue "Hey, but there is an attribute", but in fact it doesn't work the way you expect and may produce panics in safe Rust:

https://play.rust-lang.org/?version=nightly&mode=debug&edition=2018&gist=dd880f555dff38824b094dd8dec2f6f2

I'd better to be replaced with num_derive:

#[repr(u8)]
#[derive(Debug, Copy, Clone, FromPrimitive)]
pub enum MsgType {

[Pzixel]

Looks like it gonna be fixed by #63

[hdhoang]

oh hey! thanks for your interest. We are very much interested in parsing the chip-to-chip message safely (#63 implements #20), but the struct-oriented scroll gave me some trouble with switching on enums. Your num_derive/num_traits suggestion might work very well indeed. I will give it a try, or maybe clone the trait into anne-key if it's no_std-incompatible.

[Pzixel]

Good to hear.

I've used scroll to parse ECMA335 CLI metadata and it worked fine for me. I'm surprised you have any troubles with it.

[hdhoang]

My main trouble is that operation in MsgType is currently parsed as-needed, thus not buying much logic-safety over variant-casting. I'll look on crates.io and your code for examples too.

The good news is num-traits works just as you suggested, hooray!

[Pzixel]

You can express lazy parsing with any kind of Lazy<T> wrapper, if you wish. However, I don't see much troubles in one pattern match even if you don't need the result. It's very cheap after all, it doesn't make any heavy magic with strings or something.