From c423ac4517667c63c8403e247176a29bda1efb78 Mon Sep 17 00:00:00 2001 From: David Brown Date: Fri, 4 Jun 2021 13:47:34 -0600 Subject: [PATCH] sim: Return abstract value from boot_go Instead of a tuple of values that is matched, return an abstract type that has methods for querying the information we need. Abstracting this will allow us to return additional information without having to change all of the code that matches against these patterns. Signed-off-by: David Brown --- sim/mcuboot-sys/src/c.rs | 34 ++++++++++++++++-- sim/src/image.rs | 75 +++++++++++++++++----------------------- 2 files changed, 63 insertions(+), 46 deletions(-) diff --git a/sim/mcuboot-sys/src/c.rs b/sim/mcuboot-sys/src/c.rs index a7c45771..8729301d 100644 --- a/sim/mcuboot-sys/src/c.rs +++ b/sim/mcuboot-sys/src/c.rs @@ -10,9 +10,39 @@ use crate::area::AreaDesc; use simflash::SimMultiFlash; use crate::api; +/// The result of an invocation of `boot_go`. This is intentionally opaque so that we can provide +/// accessors for everything we need from this. +#[derive(Debug)] +pub struct BootGoResult { + result: i32, + asserts: u8, +} + +impl BootGoResult { + /// Was this run interrupted. + pub fn interrupted(&self) -> bool { + self.result == -0x13579 + } + + /// Was this boot run successful (returned 0) + pub fn success(&self) -> bool { + self.result == 0 + } + + /// Success, but also no asserts. + pub fn success_no_asserts(&self) -> bool { + self.result == 0 && self.asserts == 0 + } + + /// Get the asserts count. + pub fn asserts(&self) -> u8 { + self.asserts + } +} + /// Invoke the bootloader on this flash device. pub fn boot_go(multiflash: &mut SimMultiFlash, areadesc: &AreaDesc, - counter: Option<&mut i32>, catch_asserts: bool) -> (i32, u8) { + counter: Option<&mut i32>, catch_asserts: bool) -> BootGoResult { for (&dev_id, flash) in multiflash.iter_mut() { api::set_flash(dev_id, flash); } @@ -42,7 +72,7 @@ pub fn boot_go(multiflash: &mut SimMultiFlash, areadesc: &AreaDesc, for &dev_id in multiflash.keys() { api::clear_flash(dev_id); } - (result, asserts) + BootGoResult { result, asserts } } pub fn boot_trailer_sz(align: u32) -> u32 { diff --git a/sim/src/image.rs b/sim/src/image.rs index 988d136e..5898560c 100644 --- a/sim/src/image.rs +++ b/sim/src/image.rs @@ -428,8 +428,7 @@ impl Images { if Caps::Bootstrap.present() { info!("Try bootstraping image in the primary"); - let (result, _) = c::boot_go(&mut flash, &self.areadesc, None, false); - if result != 0 { + if !c::boot_go(&mut flash, &self.areadesc, None, false).success() { warn!("Failed first boot"); fails += 1; } @@ -598,8 +597,7 @@ impl Images { info!("Try norevert"); // First do a normal upgrade... - let (result, _) = c::boot_go(&mut flash, &self.areadesc, None, false); - if result != 0 { + if !c::boot_go(&mut flash, &self.areadesc, None, false).success() { warn!("Failed first boot"); fails += 1; } @@ -632,8 +630,7 @@ impl Images { fails += 1; } - let (result, _) = c::boot_go(&mut flash, &self.areadesc, None, false); - if result != 0 { + if !c::boot_go(&mut flash, &self.areadesc, None, false).success() { warn!("Failed second boot"); fails += 1; } @@ -668,8 +665,7 @@ impl Images { info!("Try no downgrade"); // First, do a normal upgrade. - let (result, _) = c::boot_go(&mut flash, &self.areadesc, None, false); - if result != 0 { + if !c::boot_go(&mut flash, &self.areadesc, None, false).success() { warn!("Failed first boot"); fails += 1; } @@ -706,8 +702,7 @@ impl Images { } // Run the bootloader... - let (result, _) = c::boot_go(&mut flash, &self.areadesc, None, false); - if result != 0 { + if !c::boot_go(&mut flash, &self.areadesc, None, false).success() { warn!("Failed first boot"); fails += 1; } @@ -755,8 +750,7 @@ impl Images { } // Run the bootloader... - let (result, _) = c::boot_go(&mut flash, &self.areadesc, None, false); - if result != 0 { + if !c::boot_go(&mut flash, &self.areadesc, None, false).success() { warn!("Failed first boot"); fails += 1; } @@ -792,8 +786,7 @@ impl Images { self.mark_upgrades(&mut flash, 1); // Run the bootloader... - let (result, _) = c::boot_go(&mut flash, &self.areadesc, None, false); - if result != 0 { + if !c::boot_go(&mut flash, &self.areadesc, None, false).success() { warn!("Failed first boot"); fails += 1; } @@ -840,15 +833,15 @@ impl Images { self.mark_permanent_upgrades(&mut flash, 1); self.mark_bad_status_with_rate(&mut flash, 0, 1.0); - let (result, asserts) = c::boot_go(&mut flash, &self.areadesc, None, true); - if result != 0 { + let result = c::boot_go(&mut flash, &self.areadesc, None, true); + if !result.success() { warn!("Failed!"); fails += 1; } // Failed writes to the marked "bad" region don't assert anymore. // Any detected assert() is happening in another part of the code. - if asserts != 0 { + if result.asserts() != 0 { warn!("At least one assert() was called"); fails += 1; } @@ -866,8 +859,7 @@ impl Images { info!("validate primary slot enabled; \ re-run of boot_go should just work"); - let (result, _) = c::boot_go(&mut flash, &self.areadesc, None, false); - if result != 0 { + if !c::boot_go(&mut flash, &self.areadesc, None, false).success() { warn!("Failed!"); fails += 1; } @@ -899,7 +891,7 @@ impl Images { self.mark_bad_status_with_rate(&mut flash, 0, 0.5); // Should not fail, writing to bad regions does not assert - let (_, asserts) = c::boot_go(&mut flash, &self.areadesc, Some(&mut count), true); + let asserts = c::boot_go(&mut flash, &self.areadesc, Some(&mut count), true).asserts(); if asserts != 0 { warn!("At least one assert() was called"); fails += 1; @@ -908,7 +900,7 @@ impl Images { self.reset_bad_status(&mut flash, 0); info!("Resuming an interrupted swap operation"); - let (_, asserts) = c::boot_go(&mut flash, &self.areadesc, None, true); + let asserts = c::boot_go(&mut flash, &self.areadesc, None, true).asserts(); // This might throw no asserts, for large sector devices, where // a single failure writing is indistinguishable from no failure, @@ -935,7 +927,7 @@ impl Images { self.mark_bad_status_with_rate(&mut flash, 0, 1.0); // This is expected to fail while writing to bad regions... - let (_, asserts) = c::boot_go(&mut flash, &self.areadesc, None, true); + let asserts = c::boot_go(&mut flash, &self.areadesc, None, true).asserts(); if asserts == 0 { warn!("No assert() detected"); fails += 1; @@ -995,18 +987,18 @@ impl Images { let mut counter = stop.unwrap_or(0); let (first_interrupted, count) = match c::boot_go(&mut flash, &self.areadesc, Some(&mut counter), false) { - (-0x13579, _) => (true, stop.unwrap()), - (0, _) => (false, -counter), - (x, _) => panic!("Unknown return: {}", x), + x if x.interrupted() => (true, stop.unwrap()), + x if x.success() => (false, -counter), + x => panic!("Unknown return: {:?}", x), }; counter = 0; if first_interrupted { // fl.dump(); match c::boot_go(&mut flash, &self.areadesc, Some(&mut counter), false) { - (-0x13579, _) => panic!("Shouldn't stop again"), - (0, _) => (), - (x, _) => panic!("Unknown return: {}", x), + x if x.interrupted() => panic!("Shouldn't stop again"), + x if x.success() => (), + x => panic!("Unknown return: {:?}", x), } } @@ -1019,7 +1011,7 @@ impl Images { // fl.write_file("image0.bin").unwrap(); for i in 0 .. count { info!("Running boot pass {}", i + 1); - assert_eq!(c::boot_go(&mut flash, &self.areadesc, None, false), (0, 0)); + assert!(c::boot_go(&mut flash, &self.areadesc, None, false).success_no_asserts()); } flash } @@ -1029,8 +1021,7 @@ impl Images { let mut fails = 0; let mut counter = stop; - let (x, _) = c::boot_go(&mut flash, &self.areadesc, Some(&mut counter), false); - if x != -0x13579 { + if !c::boot_go(&mut flash, &self.areadesc, Some(&mut counter), false).interrupted() { warn!("Should have stopped test at interruption point"); fails += 1; } @@ -1042,8 +1033,7 @@ impl Images { fails += 1; } - let (x, _) = c::boot_go(&mut flash, &self.areadesc, None, false); - if x != 0 { + if !c::boot_go(&mut flash, &self.areadesc, None, false).success() { warn!("Should have finished test upgrade"); fails += 1; } @@ -1071,14 +1061,12 @@ impl Images { // Do Revert let mut counter = stop; - let (x, _) = c::boot_go(&mut flash, &self.areadesc, Some(&mut counter), false); - if x != -0x13579 { + if !c::boot_go(&mut flash, &self.areadesc, Some(&mut counter), false).interrupted() { warn!("Should have stopped revert at interruption point"); fails += 1; } - let (x, _) = c::boot_go(&mut flash, &self.areadesc, None, false); - if x != 0 { + if !c::boot_go(&mut flash, &self.areadesc, None, false).success() { warn!("Should have finished revert upgrade"); fails += 1; } @@ -1105,8 +1093,7 @@ impl Images { fails += 1; } - let (x, _) = c::boot_go(&mut flash, &self.areadesc, None, false); - if x != 0 { + if !c::boot_go(&mut flash, &self.areadesc, None, false).success() { warn!("Should have finished 3rd boot"); fails += 1; } @@ -1136,17 +1123,17 @@ impl Images { let reset_counter = rng.gen_range(1, remaining_ops / 2); let mut counter = reset_counter; match c::boot_go(&mut flash, &self.areadesc, Some(&mut counter), false) { - (0, _) | (-0x13579, _) => (), - (x, _) => panic!("Unknown return: {}", x), + x if x.interrupted() => (), + x => panic!("Unknown return: {:?}", x), } remaining_ops -= reset_counter; *reset = reset_counter; } match c::boot_go(&mut flash, &self.areadesc, None, false) { - (-0x13579, _) => panic!("Should not be have been interrupted!"), - (0, _) => (), - (x, _) => panic!("Unknown return: {}", x), + x if x.interrupted() => panic!("Should not be have been interrupted!"), + x if x.success() => (), + x => panic!("Unknown return: {:?}", x), } (flash, resets)