Audit panics

Author: kurtjd

src/pdo/mod.rs

@@ -54,12 +54,15 @@ impl From<u32> for PdoKind {
 
 impl From<u8> for PdoKind {
     fn from(value: u8) -> Self {
+        // NOTE: If this mask changes, the panic safety comment below must be reevaluated
         const PDO_KIND_MASK: u8 = 0x3;
         match value & PDO_KIND_MASK {
             0x0 => PdoKind::Fixed,
             0x1 => PdoKind::Battery,
             0x2 => PdoKind::Variable,
             0x3 => PdoKind::Augmented,
+            // Panic safety: This will never panic if the mask above does not change
+            #[allow(clippy::unreachable)]
             _ => unreachable!(),
         }
     }

src/pdo/rdo.rs

@@ -22,17 +22,19 @@ pub enum Rdo {
 
 impl Rdo {
     /// Create a new RDO from the raw data and the corresponding PDO
-    pub fn for_pdo(rdo: u32, pdo: impl Common) -> Self {
-        match pdo.kind() {
+    ///
+    /// Returns `None` if `pdo` does not contain an APDO kind
+    pub fn for_pdo(rdo: u32, pdo: impl Common) -> Option<Self> {
+        Some(match pdo.kind() {
             PdoKind::Fixed => Rdo::Fixed(FixedVarRaw(rdo).into()),
             PdoKind::Variable => Rdo::Variable(FixedVarRaw(rdo).into()),
             PdoKind::Battery => Rdo::Battery(BatteryRaw(rdo).into()),
-            PdoKind::Augmented => match pdo.apdo_kind().unwrap() {
+            PdoKind::Augmented => match pdo.apdo_kind()? {
                 ApdoKind::SprPps => Rdo::Pps(PpsRaw(rdo).into()),
                 ApdoKind::EprAvs => Rdo::Pps(PpsRaw(rdo).into()),
                 ApdoKind::SprAvs => Rdo::Avs(AvsRaw(rdo).into()),
             },
-        }
+        })
     }
 }
 
@@ -375,7 +377,8 @@ mod tests {
                 voltage_mv: 0,
                 operational_current_ma: 0,
             }),
-        );
+        )
+        .unwrap();
         let expected = Rdo::Fixed(FixedVarData {
             object_position: 3,
             capability_mismatch: true,
@@ -401,7 +404,8 @@ mod tests {
                 min_voltage_mv: 0,
                 operational_current_ma: 0,
             }),
-        );
+        )
+        .unwrap();
         let expected = Rdo::Variable(FixedVarData {
             object_position: 3,
             capability_mismatch: true,
@@ -427,7 +431,8 @@ mod tests {
                 min_voltage_mv: 0,
                 operational_power_mw: 0,
             }),
-        );
+        )
+        .unwrap();
         let expected = Rdo::Battery(BatteryData {
             object_position: 3,
             capability_mismatch: true,
@@ -453,7 +458,8 @@ mod tests {
                 min_voltage_mv: 0,
                 max_current_ma: 0,
             })),
-        );
+        )
+        .unwrap();
         let expected = Rdo::Pps(PpsData {
             object_position: 3,
             capability_mismatch: true,
@@ -478,7 +484,8 @@ mod tests {
                 max_current_15v_ma: 0,
                 max_current_20v_ma: 0,
             })),
-        );
+        )
+        .unwrap();
         let expected = Rdo::Avs(AvsData {
             object_position: 3,
             capability_mismatch: true,

src/pdo/sink.rs

@@ -152,12 +152,15 @@ pub enum FrsRequiredCurrent {
 
 impl From<u8> for FrsRequiredCurrent {
     fn from(value: u8) -> Self {
+        // NOTE: If this mask changes, the panic safety comment below must be reevaluated
         const FRS_REQUIRED_CURRENT_MASK: u8 = 0x3;
         match value & FRS_REQUIRED_CURRENT_MASK {
             0 => FrsRequiredCurrent::None,
             1 => FrsRequiredCurrent::Default,
             2 => FrsRequiredCurrent::Current1A5,
             3 => FrsRequiredCurrent::Current3A,
+            // Panic safety: This will never panic if the mask above does not change
+            #[allow(clippy::unreachable)]
             _ => unreachable!(),
         }
     }

src/pdo/source.rs

@@ -146,14 +146,17 @@ pub enum PeakCurrent {
     Pct150,
 }
 
-const PEAK_CURRENT_MASK: u8 = 0x3;
 impl From<u8> for PeakCurrent {
     fn from(value: u8) -> Self {
+        // NOTE: If this mask changes, the panic safety comment below must be reevaluated
+        const PEAK_CURRENT_MASK: u8 = 0x3;
         match value & PEAK_CURRENT_MASK {
             0x0 => PeakCurrent::Pct100,
             0x1 => PeakCurrent::Pct110,
             0x2 => PeakCurrent::Pct125,
             0x3 => PeakCurrent::Pct150,
+            // Panic safety: This will never panic if the mask above does not change
+            #[allow(clippy::unreachable)]
             _ => unreachable!(),
         }
     }

src/ucsi/lpm/get_alternate_modes.rs

@@ -53,10 +53,8 @@ impl defmt::Format for ArgsRaw {
 pub struct Args(ArgsRaw);
 
 impl Args {
-    pub fn recipient(&self) -> Recipient {
-        let recipient: Result<Recipient, _> = self.0.recipient().try_into();
-        // Won't panic, validated in try_from
-        recipient.unwrap()
+    pub fn recipient(&self) -> Result<Recipient, InvalidRecipient> {
+        self.0.recipient().try_into()
     }
 
     pub fn set_recipient(&mut self, recipient: Recipient) -> &mut Self {

src/ucsi/lpm/get_cable_property.rs

@@ -68,6 +68,8 @@ impl From<u16> for SpeedSupported {
             0x1 => SpeedSupported::Kbps(raw.value()),
             0x2 => SpeedSupported::Mbps(raw.value()),
             0x3 => SpeedSupported::Gbps(raw.value()),
+            // Panic safety: `units` is constrained to 2 bits via bitfield so this will never panic
+            #[allow(clippy::unreachable)]
             _ => unreachable!(),
         }
     }
@@ -126,6 +128,8 @@ impl From<u8> for PlugEndType {
             0x1 => PlugEndType::TypeB,
             0x2 => PlugEndType::TypeC,
             0x3 => PlugEndType::Other,
+            // Panic safety: This is trivially guaranteed to not panic
+            #[allow(clippy::unreachable)]
             _ => unreachable!(),
         }
     }

src/ucsi/lpm/get_connector_status.rs

@@ -704,8 +704,11 @@ impl From<ResponseData> for [u8; RESPONSE_DATA_LEN] {
             raw.set_partner_flags(status.partner_flags.into());
             raw.set_partner_type(status.partner_type as u8);
 
-            if status.rdo.is_some_and(|rdo| rdo != 0) {
-                raw.set_rdo(status.rdo.unwrap());
+            // Note: Can be collapsed to a let chain when this crate is updated to 2024 edition
+            if let Some(rdo) = status.rdo {
+                if rdo != 0 {
+                    raw.set_rdo(rdo)
+                }
             }
 
             if let Some(battery_charging_status) = status.battery_charging_status {

src/ucsi/lpm/get_pdos.rs

@@ -171,9 +171,8 @@ impl Args {
         self
     }
 
-    pub fn source_capability_type(&self) -> SourceCapabilityType {
-        // Won't panic, validated in try_from
-        self.0.source_capability_type().try_into().unwrap()
+    pub fn source_capability_type(&self) -> Result<SourceCapabilityType, InvalidSourceCapabilityType> {
+        self.0.source_capability_type().try_into()
     }
 
     pub fn set_source_capability_type(&mut self, source_capabilities_type: SourceCapabilityType) -> &mut Self {
@@ -227,13 +226,19 @@ pub struct ResponseData {
 impl ResponseData {
     /// Iterator over valid PDOs
     pub fn iter(&self) -> impl ExactSizeIterator<Item = u32> + '_ {
+        // NOTE: If this changes the panic safety comment below should be revisited
         let last_pdo = self.raw.iter().position(|&pdo| pdo == 0).unwrap_or(self.raw.len());
+        // Panic safety: `last_pdo` will always be in bounds
+        #[allow(clippy::indexing_slicing)]
         self.raw.as_slice()[..last_pdo].iter().copied()
     }
 
     /// Mutable iterator over valid PDOs
     pub fn iter_mut(&mut self) -> impl ExactSizeIterator<Item = &mut u32> + '_ {
+        // NOTE: If this changes the panic safety comment below should be revisited
         let last_pdo = self.raw.iter().position(|&pdo| pdo == 0).unwrap_or(self.raw.len());
+        // Panic safety: `last_pdo` will always be in bounds
+        #[allow(clippy::indexing_slicing)]
         self.raw.as_mut_slice()[..last_pdo].iter_mut()
     }
 }

src/ucsi/lpm/mod.rs

@@ -198,7 +198,11 @@ impl<T: PortId> Encode for Command<T> {
 
 impl<T: PortId> Decode<CommandHeader> for Command<T> {
     fn decode<D: Decoder<Context = CommandHeader>>(decoder: &mut D) -> Result<Self, DecodeError> {
-        match decoder.context().command() {
+        match decoder
+            .context()
+            .command()
+            .map_err(|_| DecodeError::Other("Invalid command"))?
+        {
             CommandType::ConnectorReset => {
                 let connector_number = ConnectorNumberRaw::decode(decoder)?.connector_number();
                 // Don't actually have any args, but need to consume command padding

src/ucsi/lpm/set_power_level.rs

@@ -135,9 +135,8 @@ impl Args {
         }
     }
 
-    pub fn type_c_current(&self) -> Current {
-        let current: Result<Current, _> = Current::try_from(self.0.type_c_current());
-        current.unwrap() // Won't panic, validated in try_from
+    pub fn type_c_current(&self) -> Result<Current, InvalidCurrent> {
+        Current::try_from(self.0.type_c_current())
     }
 
     pub fn set_type_c_current(&mut self, type_c_current: Current) -> &mut Self {