Work with the epoch value as a u32. Do not use ImageHeader epoch field.

Author: lzrd

Cargo.lock

@@ -34,7 +34,7 @@ pub enum Command {
         version: String,
 
         #[clap(short, long)]
-        epoch: Option<String>,
+        epoch: Option<u32>,
 
         #[clap(short, long)]
         force: bool,
@@ -151,10 +151,9 @@ fn main() -> Result<()> {
                 }
             }
             if no_defaults {
-                archive.write_version_to_caboose(&version, epoch.as_ref())?;
+                archive.write_version_to_caboose(&version, epoch)?;
             } else {
-                archive
-                    .write_default_caboose(Some(&version), epoch.as_ref())?;
+                archive.write_default_caboose(Some(&version), epoch)?;
             }
             archive.overwrite()?;
         }
@@ -173,36 +172,22 @@ fn main() -> Result<()> {
             // or have no caboose, or have no caboose with an `EPOC` value.
             // The default epoch value is zero.
 
-            let (version, caboose_epoch) =
-                if let Ok(caboose) = archive.read_caboose() {
-                    let version = match caboose.version() {
-                        Ok(vers) => {
-                            std::str::from_utf8(vers).unwrap_or("").to_string()
-                        }
-                        Err(_) => "".to_string(),
-                    };
-                    let epoch =
-                        std::str::from_utf8(caboose.epoch().unwrap_or(&[b'0']))
-                            .unwrap_or("0")
-                            .parse::<u32>()
-                            .unwrap_or(0u32);
-                    (version.clone(), epoch)
-                } else {
-                    // No caboose
-                    ("".to_string(), 0u32)
+            let (version, epoch) = if let Ok(caboose) = archive.read_caboose() {
+                let version = match caboose.version() {
+                    Ok(vers) => {
+                        std::str::from_utf8(vers).unwrap_or("").to_string()
+                    }
+                    Err(_) => "".to_string(),
                 };
-            let header_epoch = archive.image.image_header_epoch().unwrap_or(0);
-
-            println!("Version: \"{}\"", version);
-            if header_epoch == caboose_epoch {
-                println!("Epoch: {}", header_epoch);
+                let epoch = caboose.epoch_u32().unwrap_or(0u32);
+                (version.clone(), epoch)
             } else {
-                println!("Epoch: {}", 0u32);
-                bail!(format!(
-                    "Epoch mismatch: ImageHeader({}) != Caboose({})",
-                    header_epoch, caboose_epoch
-                ));
-            }
+                // No caboose
+                ("".to_string(), 0u32)
+            };
+
+            println!("Version: \"{version}\"");
+            println!("Epoch: {epoch}");
         }
         Command::ReplaceImage { image } => {
             let contents = std::fs::read(&image).with_context(|| {

hubedit/src/main.rs

@@ -57,7 +57,7 @@ fn header_offset(elf: &object::read::File) -> Result<u64, Error> {
     Err(Error::MissingHeader)
 }
 
-fn add_image_header(path: PathBuf, epoch: u32) -> Result<Vec<u8>, Error> {
+fn add_image_header(path: PathBuf) -> Result<Vec<u8>, Error> {
     // We can't construct an ELF object from a mutable reference so we
     // work on the file path and return the bytes
     let mut f = std::fs::read(path).map_err(Error::FileReadError)?;
@@ -72,11 +72,7 @@ fn add_image_header(path: PathBuf, epoch: u32) -> Result<Vec<u8>, Error> {
     let offset = header_offset(&elf)?;
     drop(elf);
 
-    let header = header::ImageHeader {
-        total_image_len: len as u32,
-        epoch,
-        ..Default::default()
-    };
+    let header = header::ImageHeader::new(len as usize);
 
     header
         .write_to_prefix(&mut f[(offset as usize)..])
@@ -92,9 +88,7 @@ pub fn bootleby_to_archive(
     gitc: String,
     epoc: Option<u32>,
 ) -> Result<Vec<u8>, Error> {
-    let image_header_epoch: u32 = epoc.unwrap_or(0u32);
-    let epoc = image_header_epoch.to_string();
-    let f = add_image_header(path, image_header_epoch)?;
+    let f = add_image_header(path)?;
 
     let img = RawHubrisImage::from_generic_elf(&f)?;
 
@@ -109,7 +103,9 @@ pub fn bootleby_to_archive(
         chip = "lpc55"
         epoch = {}
         "#,
-        name, board, epoc
+        name,
+        board,
+        epoc.unwrap_or(0u32)
     );
 
     archive.add_file("elf/kernel", &f)?;

hubtools/src/bootleby.rs

@@ -52,6 +52,18 @@ impl Caboose {
         self.get_tag(tags::EPOC)
     }
 
+    /// Interpret the `EPOC` value as a u32 if present and well formed.
+    pub fn epoch_u32(&self) -> Option<u32> {
+        if let Ok(epoc) = self.epoch() {
+            if let Ok(epoc_str) = std::str::from_utf8(epoc) {
+                if let Ok(number) = epoc_str.parse::<u32>() {
+                    return Some(number);
+                }
+            }
+        }
+        None
+    }
+
     fn get_tag(&self, tag: [u8; 4]) -> Result<&[u8], CabooseError> {
         use tlvc::TlvcReader;
         let mut reader = TlvcReader::begin(self.as_slice())
@@ -99,7 +111,7 @@ pub struct CabooseBuilder {
     name: Option<String>,
     version: Option<String>,
     sign: Option<String>,
-    epoch: Option<String>,
+    epoch: Option<u32>,
 }
 
 impl CabooseBuilder {
@@ -128,19 +140,8 @@ impl CabooseBuilder {
         self
     }
 
-    pub fn epoch<S: Into<String>>(mut self, epoch: S) -> Self {
-        let epoch: String = epoch.into();
-        match epoch.len() {
-            0 => self.epoch = Some("0".to_string()),
-            _ => {
-                if let Ok(epoch) = epoch.parse::<u32>() {
-                    self.epoch = Some(format!("{epoch}").to_owned());
-                } else {
-                    // Any invalid value is treated as zero
-                    self.epoch = Some("0".to_string())
-                }
-            }
-        }
+    pub fn epoch(mut self, epoch: u32) -> Self {
+        self.epoch = Some(epoch);
         self
     }
 
@@ -152,7 +153,7 @@ impl CabooseBuilder {
             (tags::NAME, self.name),
             (tags::VERS, self.version),
             (tags::SIGN, self.sign),
-            (tags::EPOC, self.epoch),
+            (tags::EPOC, self.epoch.map(|e| e.to_string())),
         ] {
             let Some(value) = maybe_value else {
                 continue;
@@ -224,7 +225,7 @@ mod tests {
             .board("bar")
             .name("fizz")
             .version("buzz")
-            .epoch("")
+            .epoch(0)
             .build();
         assert_eq!(caboose.git_commit(), Ok("foo".as_bytes()));
         assert_eq!(caboose.board(), Ok("bar".as_bytes()));
@@ -233,29 +234,30 @@ mod tests {
         assert_eq!(caboose.epoch(), Ok("0".as_bytes()));
     }
 
+    #[test]
+    fn builder_can_make_caboose_with_zero_epoch() {
+        let caboose = CabooseBuilder::default().epoch(0).build();
+        assert_eq!(caboose.epoch(), Ok("0".as_bytes()));
+    }
+
     #[test]
     fn builder_can_make_caboose_with_non_zero_epoch() {
-        let caboose = CabooseBuilder::default().epoch("1234567890").build();
+        let caboose = CabooseBuilder::default().epoch(1234567890).build();
         assert_eq!(caboose.epoch(), Ok("1234567890".as_bytes()));
     }
 
     #[test]
-    fn builder_will_normalize_empty_epoch() {
-        let caboose = CabooseBuilder::default().epoch("").build();
-        assert_eq!(caboose.epoch(), Ok("0".as_bytes()));
+    fn builder_missing_tag_epoc() {
+        let caboose = CabooseBuilder::default().build();
+        assert_eq!(
+            caboose.epoch(),
+            Err(CabooseError::MissingTag { tag: tags::EPOC })
+        );
     }
 
     #[test]
     fn builder_will_normalize_short_epoch() {
-        let caboose = CabooseBuilder::default().epoch("1234").build();
+        let caboose = CabooseBuilder::default().epoch(1234).build();
         assert_eq!(caboose.epoch(), Ok("1234".as_bytes()));
     }
-
-    #[test]
-    fn builder_will_convert_u64_epoch_to_zero() {
-        let caboose = CabooseBuilder::default()
-            .epoch("4294967296") // u32::MAX_VALUE + 1
-            .build();
-        assert_eq!(caboose.epoch(), Ok("0".as_bytes()));
-    }
 }