From 23e8186687af8dfffafb9a67ba6ef628226543b1 Mon Sep 17 00:00:00 2001
From: Ben Stoltz <ben.stoltz@oxidecomputer.com>
Date: Tue, 9 Jul 2024 12:50:02 -0700
Subject: [PATCH] Changes from PR review

---
 drv/lpc55-update-server/src/images.rs | 24 +++++++++---------------
 drv/lpc55-update-server/src/main.rs   |  5 +++--
 2 files changed, 12 insertions(+), 17 deletions(-)

diff --git a/drv/lpc55-update-server/src/images.rs b/drv/lpc55-update-server/src/images.rs
index 4a69baafe..fc8a3d148 100644
--- a/drv/lpc55-update-server/src/images.rs
+++ b/drv/lpc55-update-server/src/images.rs
@@ -19,7 +19,6 @@ use crate::{
 };
 use abi::{ImageHeader, CABOOSE_MAGIC, HEADER_MAGIC};
 use core::ops::Range;
-use drv_lpc55_flash::BYTES_PER_FLASH_PAGE;
 use drv_lpc55_update_api::{RawCabooseError, RotComponent, SlotId};
 use drv_update_api::UpdateError;
 use zerocopy::{AsBytes, FromBytes};
@@ -91,7 +90,7 @@ pub fn flash_range(component: RotComponent, slot: SlotId) -> FlashRange {
 }
 
 /// Does (component, slot) refer to the currently running Hubris image?
-pub fn same_image(component: RotComponent, slot: SlotId) -> bool {
+pub fn is_current_hubris_image(component: RotComponent, slot: SlotId) -> bool {
     // Safety: We are trusting the linker.
     flash_range(component, slot).store.start
         == unsafe { &__this_image } as *const _ as u32
@@ -293,7 +292,7 @@ pub fn caboose_slice(
     }
 }
 
-// Accessor keeps the implementation details of ImageAccess private
+/// Accessor keeps the implementation details of ImageAccess private
 enum Accessor<'a> {
     // Flash driver, flash device range
     Flash {
@@ -372,8 +371,6 @@ impl ImageAccess<'_> {
         component: RotComponent,
         slot: SlotId,
     ) -> ImageAccess<'a> {
-        assert!((buffer.len() % BYTES_PER_FLASH_PAGE) == 0);
-        assert!(((buffer.as_ptr() as u32) % U32_SIZE) == 0);
         let span = flash_range(component, slot);
         ImageAccess {
             accessor: Accessor::_Hybrid {
@@ -459,9 +456,8 @@ impl ImageAccess<'_> {
         let len = buffer.len() as u32;
         match &self.accessor {
             Accessor::Flash { flash, span } => {
-                let start = offset.saturating_add(span.store.start);
-                let end =
-                    offset.saturating_add(span.store.start).saturating_add(len);
+                let start = span.store.start.saturating_add(offset);
+                let end = start.saturating_add(len);
                 if span.store.contains(&start)
                     && (span.store.start..=span.store.end).contains(&end)
                 {
@@ -503,14 +499,12 @@ impl ImageAccess<'_> {
                 }
                 // Transfer data from the flash-backed portion of the image.
                 if remainder > 0 {
-                    let start =
-                        offset.saturating_add(span.store.start as usize);
-                    let end = start.saturating_add(remainder);
-                    if span.store.contains(&(start as u32))
-                        && (span.store.start..=span.store.end)
-                            .contains(&(end as u32))
+                    let start = span.store.start.saturating_add(offset as u32);
+                    let end = start.saturating_add(remainder as u32);
+                    if span.store.contains(&start)
+                        && (span.store.start..=span.store.end).contains(&end)
                     {
-                        indirect_flash_read(flash, start as u32, buffer)?;
+                        indirect_flash_read(flash, start, buffer)?;
                     } else {
                         return Err(UpdateError::OutOfBounds);
                     }
diff --git a/drv/lpc55-update-server/src/main.rs b/drv/lpc55-update-server/src/main.rs
index 0e7b4f607..48d9f4a33 100644
--- a/drv/lpc55-update-server/src/main.rs
+++ b/drv/lpc55-update-server/src/main.rs
@@ -33,7 +33,8 @@ use zerocopy::AsBytes;
 
 mod images;
 use crate::images::{
-    caboose_slice, flash_range, same_image, ImageAccess, HEADER_BLOCK,
+    caboose_slice, flash_range, is_current_hubris_image, ImageAccess,
+    HEADER_BLOCK,
 };
 
 const PAGE_SIZE: u32 = BYTES_PER_FLASH_PAGE as u32;
@@ -1143,7 +1144,7 @@ fn do_block_write(
     let page_num = block_num as u32;
 
     // Can only update opposite image
-    if same_image(component, slot) {
+    if is_current_hubris_image(component, slot) {
         return Err(UpdateError::RunningImage);
     }