Minimize panic code paths

.github/workflows/check.yml

@@ -36,33 +36,6 @@ jobs:
       - name: cargo fmt --check
         run: cargo fmt --check
 
-  clippy:
-    runs-on: ubuntu-latest
-    name: ${{ matrix.toolchain }} / clippy
-    permissions:
-      contents: read
-      checks: write
-    strategy:
-      fail-fast: false
-      matrix:
-        # Get early warning of new lints which are regularly introduced in beta channels.
-        toolchain: [stable, beta]
-    steps:
-      - uses: actions/checkout@v4
-        with:
-          submodules: true
-      - name: Install ${{ matrix.toolchain }}
-        uses: dtolnay/rust-toolchain@master
-        with:
-          toolchain: ${{ matrix.toolchain }}
-          components: clippy
-      - name: cargo clippy
-        uses: giraffate/clippy-action@v1
-        with:
-          reporter: 'github-pr-check'
-          clippy_flags: -- -F clippy::suspicious -F clippy::correctness -F clippy::perf -F clippy::style
-          github_token: ${{ secrets.GITHUB_TOKEN }}
-
   # Enable once we have a released crate
   # semver:
   #   runs-on: ubuntu-latest
@@ -106,12 +79,16 @@ jobs:
           submodules: true
       - name: Install stable
         uses: dtolnay/rust-toolchain@stable
+        with:
+          components: clippy
       - name: cargo install cargo-hack
         uses: taiki-e/install-action@cargo-hack
       # intentionally no target specifier; see https://github.com/jonhoo/rust-ci-conf/pull/4
       # --feature-powerset runs for every combination of features
       - name: cargo hack
         run: cargo hack --feature-powerset --mutually-exclusive-features=log,defmt check
+      - name : cargo hack clippy
+        run: cargo hack --feature-powerset --mutually-exclusive-features=log,defmt clippy -- -Dwarnings
 
   deny:
     # cargo-deny checks licenses, advisories, sources, and bans for

Cargo.toml

@@ -16,4 +16,21 @@ defmt = [
 ]
 log = [
     "dep:log",
-]
+]
+
+[lints.rust]
+unsafe_code = "forbid"
+
+[lints.clippy]
+correctness = "deny"
+expect_used = "deny"
+indexing_slicing = "deny"
+panic = "deny"
+panic_in_result_fn = "deny"
+perf = "deny"
+suspicious = "deny"
+style = "deny"
+todo = "deny"
+unimplemented = "deny"
+unreachable = "deny"
+unwrap_used = "deny"

src/host.rs

@@ -99,7 +99,8 @@ impl<W: CfuWriterAsync> CfuUpdateContent<W> for CfuUpdater {
         let remainder = total_bytes % chunk_size;
 
         // Read and process data in chunks so as to not over-burden memory resources
-        let mut resp = FwUpdateContentResponse::new(0, CfuUpdateContentResponseStatus::ErrorInvalid);
+        let mut resp: FwUpdateContentResponse =
+            FwUpdateContentResponse::new(0, CfuUpdateContentResponseStatus::ErrorInvalid);
         for i in 0..num_chunks {
             let mut chunk = [0u8; DEFAULT_DATA_LENGTH];
             let address_offset = i * DEFAULT_DATA_LENGTH + base_offset;
@@ -120,7 +121,12 @@ impl<W: CfuWriterAsync> CfuUpdateContent<W> for CfuUpdater {
                 }
                 _ => {
                     image
-                        .get_bytes_for_chunk(&mut chunk[..remainder], address_offset)
+                        .get_bytes_for_chunk(
+                            chunk
+                                .get_mut(0..remainder)
+                                .ok_or(CfuProtocolError::WriterError(CfuWriterError::Other))?,
+                            address_offset,
+                        )
                         .await
                         .map_err(|_| CfuProtocolError::WriterError(CfuWriterError::StorageError))?;
                     self.process_last_data_block(writer, chunk, i).await

src/protocol_definitions.rs

@@ -169,8 +169,10 @@ impl Default for FwVerComponentInfo {
 }
 
 // Convert to bytes
-impl From<&GetFwVersionResponse> for [u8; 60] {
-    fn from(response: &GetFwVersionResponse) -> Self {
+impl TryFrom<&GetFwVersionResponse> for [u8; 60] {
+    type Error = ConversionError;
+
+    fn try_from(response: &GetFwVersionResponse) -> Result<Self, Self::Error> {
         let mut bytes = [0u8; 60];
 
         // Serialize header
@@ -181,32 +183,48 @@ impl From<&GetFwVersionResponse> for [u8; 60] {
         // Serialize component_info
         let mut offset = 4;
         for i in 0..response.header.component_count as usize {
-            let component = &response.component_info[i];
-            bytes[offset] = component.packed_byte;
-            bytes[offset + 1] = component.component_id;
-            bytes[offset + 2..offset + 4].copy_from_slice(&component.vendor_specific1.to_le_bytes());
-            bytes[offset + 4] = component.fw_version.major;
-            bytes[offset + 5..offset + 7].copy_from_slice(&component.fw_version.minor.to_le_bytes());
-            bytes[offset + 7] = component.fw_version.variant;
+            let component = &response
+                .component_info
+                .get(i)
+                .ok_or(ConversionError::ByteConversionError)?;
+            *bytes.get_mut(offset).ok_or(ConversionError::ByteConversionError)? = component.packed_byte;
+            *bytes.get_mut(offset + 1).ok_or(ConversionError::ByteConversionError)? = component.component_id;
+            bytes
+                .get_mut(offset + 2..offset + 4)
+                .ok_or(ConversionError::ByteConversionError)?
+                .copy_from_slice(&component.vendor_specific1.to_le_bytes());
+            *bytes.get_mut(offset + 4).ok_or(ConversionError::ByteConversionError)? = component.fw_version.major;
+            bytes
+                .get_mut(offset + 5..offset + 7)
+                .ok_or(ConversionError::ByteConversionError)?
+                .copy_from_slice(&component.fw_version.minor.to_le_bytes());
+            *bytes.get_mut(offset + 7).ok_or(ConversionError::ByteConversionError)? = component.fw_version.variant;
             offset += 8;
         }
 
-        bytes
+        Ok(bytes)
     }
 }
 
 // Convert from bytes
 impl TryFrom<&[u8; 60]> for GetFwVersionResponse {
     type Error = ConversionError;
 
+    #[allow(clippy::indexing_slicing)] // static_check and fixed size array guarantees indexing is safe
     fn try_from(bytes: &[u8; 60]) -> Result<Self, Self::Error> {
+        const _: () = assert!(MAX_CMPT_COUNT * 8 + 4 <= 60, "Component count exceeds maximum allowed");
+
         let component_count = bytes[0];
 
         if component_count as usize > MAX_CMPT_COUNT {
             return Err(ConversionError::ValueOutOfRange);
         }
 
-        let _reserved = u16::from_le_bytes(bytes[1..3].try_into().unwrap());
+        let _reserved = u16::from_le_bytes(
+            bytes[1..3]
+                .try_into()
+                .map_err(|_| ConversionError::ByteConversionError)?,
+        );
         let byte3 = match bytes[3] {
             0x20 => GetFwVerRespHeaderByte3::NoSpecialFlags,
             0x21 => GetFwVerRespHeaderByte3::ExtensionFlagSet,
@@ -218,9 +236,17 @@ impl TryFrom<&[u8; 60]> for GetFwVersionResponse {
         for component in component_info.iter_mut().take(component_count as usize) {
             component.packed_byte = bytes[offset];
             component.component_id = bytes[offset + 1];
-            component.vendor_specific1 = u16::from_le_bytes(bytes[offset + 2..offset + 4].try_into().unwrap());
+            component.vendor_specific1 = u16::from_le_bytes(
+                bytes[offset + 2..offset + 4]
+                    .try_into()
+                    .map_err(|_| ConversionError::ByteConversionError)?,
+            );
             component.fw_version.major = bytes[offset + 4];
-            component.fw_version.minor = u16::from_le_bytes(bytes[offset + 5..offset + 7].try_into().unwrap());
+            component.fw_version.minor = u16::from_le_bytes(
+                bytes[offset + 5..offset + 7]
+                    .try_into()
+                    .map_err(|_| ConversionError::ByteConversionError)?,
+            );
             component.fw_version.variant = bytes[offset + 7];
             offset += 8;
         }
@@ -309,12 +335,24 @@ impl TryFrom<&[u8; 32]> for FwUpdateOffer {
 
         let firmware_version = FwVersion {
             major: bytes[7],
-            minor: u16::from_le_bytes(bytes[5..7].try_into().unwrap()),
+            minor: u16::from_le_bytes(
+                bytes[5..7]
+                    .try_into()
+                    .map_err(|_| ConversionError::ByteConversionError)?,
+            ),
             variant: bytes[4],
         };
 
-        let vendor_specific = u32::from_le_bytes(bytes[8..12].try_into().unwrap());
-        let misc_and_protocol_version = u32::from_le_bytes(bytes[12..16].try_into().unwrap());
+        let vendor_specific = u32::from_le_bytes(
+            bytes[8..12]
+                .try_into()
+                .map_err(|_| ConversionError::ByteConversionError)?,
+        );
+        let misc_and_protocol_version = u32::from_le_bytes(
+            bytes[12..16]
+                .try_into()
+                .map_err(|_| ConversionError::ByteConversionError)?,
+        );
 
         Ok(FwUpdateOffer {
             component_info,
@@ -659,8 +697,16 @@ impl TryFrom<&[u8; 60]> for FwUpdateContentCommand {
     fn try_from(bytes: &[u8; 60]) -> Result<Self, Self::Error> {
         let flags = bytes[0];
         let data_length = bytes[1];
-        let sequence_num = u16::from_le_bytes(bytes[2..4].try_into().unwrap());
-        let firmware_address = u32::from_le_bytes(bytes[4..8].try_into().unwrap());
+        let sequence_num = u16::from_le_bytes(
+            bytes[2..4]
+                .try_into()
+                .map_err(|_| ConversionError::ByteConversionError)?,
+        );
+        let firmware_address = u32::from_le_bytes(
+            bytes[4..8]
+                .try_into()
+                .map_err(|_| ConversionError::ByteConversionError)?,
+        );
 
         let mut data = [0u8; DEFAULT_DATA_LENGTH];
         data.copy_from_slice(&bytes[8..]);
@@ -1156,7 +1202,7 @@ mod tests {
         };
 
         // Serialize the fwversion_response_orig to a byte array
-        let fwversion_response_serialized: [u8; 60] = (&fwversion_response_orig).into();
+        let fwversion_response_serialized: [u8; 60] = (&fwversion_response_orig).try_into().unwrap();
 
         // Deserialize the byte array back to a GetFwVersionResponse instance
         let fwversion_response_deserialized = GetFwVersionResponse::try_from(&fwversion_response_serialized);