RawError conversion is non-exhaustive

[Wtoll]

In debugging another issue I discovered that when Softdevice::enable calls the unsafe enable function on the other side of FFI and gets back a raw error code, it immediately attempts type conversion into RawError from the primitive but does so non-exhaustively. Because the raw error is coming from the other side of FFI there's no guarantee for it to actually be a valid error code, and because the type conversion is non-exhaustive, there's an opportunity to panic that won't generate an actual Result::Err to bubble up.

// softdevice.rs
...
impl Softdevice {
    pub fn enable(config: &Config) -> &'static mut Softdevice {
        if ENABLED
            .compare_exchange(false, true, Ordering::AcqRel, Ordering::Acquire)
            .is_err()
        {
            panic!("nrf_softdevice::enable() called multiple times.")
        }

        let p_clock_lf_cfg = config.clock.as_ref().map(|x| x as _).unwrap_or(ptr::null());
        // Raw error comes straight out of an unsafe block...
        let ret = unsafe { raw::sd_softdevice_enable(p_clock_lf_cfg, Some(fault_handler)) };
        // ...into a non-exhaustive type conversion, without any prior validation.
        match RawError::convert(ret) {
            Ok(()) => {}
            // Subsequently, this branch becomes unreachable, so the panic is never actually printed.
            Err(err) => panic!("sd_softdevice_enable err {:?}", err),
        }
        ...
    }
}
...

In raw_error.rs I suggest changing the derive macro on RawError to derive TryFromPrimitive instead, and use that in the RawError::convert helper function. An invalid type conversion result could then just be matched to the RawError::Unknown type so that the program doesn't simply halt without an error message.

// raw_error.rs

#[rustfmt::skip]
#[repr(u32)]
#[cfg_attr(feature = "defmt", derive(defmt::Format))]
- #[derive(Debug, PartialEq, Eq, Clone, Copy, IntoPrimitive, FromPrimitive)]
+ #[derive(Debug, PartialEq, Eq, Clone, Copy, IntoPrimitive, TryFromPrimitive)]
pub enum RawError {
    ...
}

impl RawError {
    pub fn convert(ret: u32) -> Result<(), RawError> {
        if ret == raw::NRF_SUCCESS {
            Ok(())
        } else {
-           Err(RawError::from(ret))
+           Err(RawError::try_from(ret).unwrap_or(RawError::Unknown))
        }
    }
}

Separately, if there was perchance some known error that's causing my code to be receiving a 0x20022D91 raw error code that would be super cool to know about, but I imagine there isn't, it's just a memory issue, and that I'll have to wipe everything to troubleshoot it 😩.

Thanks everyone for writing such an amazing library! It's really been a life saver and otherwise such a pleasure to use.

[alexmoon]

The softdevice should never return an error code which is not one of the defined error codes. If we do get an invalid error code, then something has gone very wrong and I think it's most appropriate to panic in that circumstance.