further changes from reviews

gateway-messages/src/sp_to_mgs.rs

@@ -252,11 +252,37 @@ pub struct SpStateV3 {
     pub power_state: PowerState,
 }
 
+type Digest256 = [u8; 32];
+
+trait HexDisplayableArray {
+    // fn display(&self) -> fmt::Display;
+    fn display(&self) -> HexStringDisplay;
+}
+
+impl HexDisplayableArray for Digest256 {
+    fn display(&self) -> HexStringDisplay<'_> {
+        HexStringDisplay(&self[..])
+    }
+}
+
+struct HexStringDisplay<'a>(&'a [u8]);
+
+impl<'a> fmt::Display for HexStringDisplay<'a> {
+    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+        write!(f, "\"")?;
+        for b in self.0.iter() {
+            write!(f, "{:02x}", b)?;
+        }
+        write!(f, "\"")?;
+        Ok(())
+    }
+}
+
 #[derive(
     Debug, Clone, Copy, PartialEq, Eq, Deserialize, Serialize, SerializedSize,
 )]
 pub struct RotImageDetails {
-    pub digest: [u8; 32],
+    pub digest: Digest256,
     pub version: ImageVersion,
 }
 
@@ -274,7 +300,7 @@ pub struct RotImageDetailsDisplay<'a>(pub &'a RotImageDetails);
 impl<'a> fmt::Display for RotImageDetailsDisplay<'a> {
     fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
         let s = self.0;
-        writeln!(f, "digest: {}, ", &Digest256Display(&s.digest))?;
+        writeln!(f, "digest: {}, ", s.digest.display())?;
         writeln!(f, "version: {:?}, ", s.version)?;
         Ok(())
     }
@@ -398,20 +424,6 @@ impl RotStateV2 {
     }
 }
 
-// Helper for displaying FWIDs from faux-mgs
-struct Digest256Display<'a>(&'a [u8; 32]);
-
-impl<'a> fmt::Display for Digest256Display<'a> {
-    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
-        write!(f, "\"")?;
-        for b in self.0.iter() {
-            write!(f, "{:02x}", b)?;
-        }
-        write!(f, "\"")?;
-        Ok(())
-    }
-}
-
 /// This class exists for faux-mgs to nicely display the Firmware ID (FWID).
 #[derive(Clone, Debug)]
 #[must_use = "this struct does nothing unless displayed"]
@@ -440,14 +452,14 @@ impl<'a> fmt::Display for RotStateV2Display<'a> {
         write!(f, "slot_a_sha3_256_digest: ")?;
         match s.slot_a_sha3_256_digest {
             Some(digest) => {
-                writeln!(f, "Some({}), ", &Digest256Display(&digest))?;
+                writeln!(f, "Some({}), ", digest.display())?;
             }
             None => writeln!(f, "None, ")?,
         };
         write!(f, "slot_b_sha3_256_digest: ")?;
         match s.slot_b_sha3_256_digest {
             Some(digest) => {
-                writeln!(f, "Some({}), ", &Digest256Display(&digest))?;
+                writeln!(f, "Some({}), ", digest.display())?;
             }
             None => writeln!(f, "None, ")?,
         };
@@ -458,7 +470,7 @@ impl<'a> fmt::Display for RotStateV2Display<'a> {
 
 impl fmt::Display for RotStateV2 {
     fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
-        writeln!(f, "{}", &RotStateV2Display(&self))?;
+        writeln!(f, "{}", &RotStateV2Display(self))?;
         Ok(())
     }
 }
@@ -467,7 +479,7 @@ impl fmt::Display for RotStateV2 {
     Debug, Clone, Copy, PartialEq, Eq, SerializedSize, Serialize, Deserialize,
 )]
 pub enum Fwid {
-    Sha3_256([u8; 32]),
+    Sha3_256(Digest256),
 }
 
 #[derive(
@@ -541,28 +553,28 @@ impl<'a> fmt::Display for RotStateV3Display<'a> {
             Fwid::Sha3_256(digest) => writeln!(
                 f,
                 "slot_a_fwid: Fwid::Sha3_256({}), ",
-                &Digest256Display(&digest)
+                digest.display()
             )?,
         }
         match s.slot_b_fwid {
             Fwid::Sha3_256(digest) => writeln!(
                 f,
                 "slot_b_fwid: Fwid::Sha3_256({}), ",
-                &Digest256Display(&digest)
+                digest.display()
             )?,
         }
         match s.stage0_fwid {
             Fwid::Sha3_256(digest) => writeln!(
                 f,
                 "stage0_fwid: Fwid::Sha3_256({}), ",
-                &Digest256Display(&digest)
+                digest.display()
             )?,
         }
         match s.stage0next_fwid {
             Fwid::Sha3_256(digest) => writeln!(
                 f,
                 "stage0next_fwid: Fwid::Sha3_256({}), ",
-                &Digest256Display(&digest)
+                digest.display()
             )?,
         }
         writeln!(f, "slot_a_status: {:?}, ", s.slot_a_status)?;
@@ -949,9 +961,6 @@ pub enum SpError {
     Sensor(SensorError),
     Vpd(VpdError),
     Watchdog(WatchdogError),
-    BlockOutOfOrder,
-    InvalidSlotIdForOperation,
-    InvalidComponent,
 }
 
 impl fmt::Display for SpError {
@@ -1064,15 +1073,6 @@ impl fmt::Display for SpError {
             Self::Sensor(e) => write!(f, "sensor: {}", e),
             Self::Vpd(e) => write!(f, "vpd: {}", e),
             Self::Watchdog(e) => write!(f, "watchdog: {}", e),
-            Self::BlockOutOfOrder => {
-                write!(f, "block written out of order")
-            }
-            Self::InvalidSlotIdForOperation => {
-                write!(f, "SlotId parameter is not valid for request")
-            }
-            Self::InvalidComponent => {
-                write!(f, "component is not supported on device")
-            }
         }
     }
 }

gateway-messages/tests/versioning/v13.rs

@@ -2,14 +2,18 @@
 // License, v. 2.0. If a copy of the MPL was not distributed with this
 // file, You can obtain one at https://mozilla.org/MPL/2.0/.
 
+//! This source file is named after the protocol version being tested,
+//! e.g. v01.rs implements tests for protocol version 1.
+//! The tested protocol version is represented by "$VERSION" below.
+//!
 //! The tests in this module check that the serialized form of messages from MGS
-//! protocol version 12 have not changed.
+//! protocol version $VERSION have not changed.
 //!
 //! If a test in this module fails, _do not change the test_! This means you
 //! have changed, deleted, or reordered an existing message type or enum
 //! variant, and you should revert that change. This will remain true until we
-//! bump the `version::MIN` to a value higher than 11, at which point these
-//! tests can be removed as we will stop supporting v11.
+//! bump the `version::MIN` to a value higher than $VERSION, at which point these
+//! tests can be removed as we will stop supporting $VERSION.
 
 use super::assert_serialized;
 use gateway_messages::Fwid;
@@ -21,6 +25,7 @@ use gateway_messages::RotStateV3;
 use gateway_messages::SerializedSize;
 use gateway_messages::SpResponse;
 use gateway_messages::SpStateV3;
+use gateway_messages::UpdateError;
 
 #[test]
 fn sp_response() {
@@ -121,3 +126,34 @@ fn rot_boot_info_v3() {
 
     assert_serialized(&mut out, &expected, &response);
 }
+
+#[test]
+fn error_enums() {
+    let mut out = [0; SpResponse::MAX_SIZE];
+
+    let response: [ImageError; 13] = [
+        ImageError::Unchecked,
+        ImageError::FirstPageErased,
+        ImageError::PartiallyProgrammed,
+        ImageError::InvalidLength,
+        ImageError::HeaderNotProgrammed,
+        ImageError::BootloaderTooSmall,
+        ImageError::BadMagic,
+        ImageError::HeaderImageSize,
+        ImageError::UnalignedLength,
+        ImageError::UnsupportedType,
+        ImageError::ResetVectorNotThumb2,
+        ImageError::ResetVector,
+        ImageError::Signature,
+    ];
+    let expected = vec![0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12];
+    assert_serialized(&mut out, &expected, &response);
+
+    let response: [UpdateError; 3] = [
+        UpdateError::BlockOutOfOrder,
+        UpdateError::InvalidComponent,
+        UpdateError::InvalidSlotIdForOperation,
+    ];
+    let expected = vec![27, 28, 29];
+    assert_serialized(&mut out, &expected, &response);
+}