commit b2613cc2460b7b3a5aad80631649c8a8594487fe
author Sam Leffler <sleffler@google.com> Wed Aug 30 07:45:16 2023 -0700
committer Sam Leffler <sleffler@google.com> Fri Sep 01 16:31:43 2023 -0700
tree e5f4dfc1221c8439d6f5007e7960fd44ce07a1f0
parent b61afa19f507e92aa3829aa633bd3f654feab1d4

tar_loader: fixups

- from_bytes does not need a mutable reference
- add has_magic method to check the magic field
- fix name to return a string w/o  embedded nul's
- rewrite parse_octal to use iterators
- rewrite find_file to use methods, to stop searching at the first
  invalid (according to has_magic) header, and to correctly check
  for end-of-flash
- change copy_file to check the offset instead of the file size
  when deciding if a file was found--zero-length files can and do exist

Bug: 294433731

Change-Id: Ic769517bfbcc51e2e8c4f1cb485d3bb6338179b4

utils/src/tar_loader.rs

diff --git a/utils/src/tar_loader.rs b/utils/src/tar_loader.rs
index 10a3024..94ff468 100644
--- a/utils/src/tar_loader.rs
+++ b/utils/src/tar_loader.rs
@@ -1,6 +1,6 @@
 // Trivial tar loader.
 
-use core::mem::transmute;
+use core::mem::{size_of, transmute};
 use core::ptr;
 use matcha_hal::dprintf;
 
@@ -54,12 +54,21 @@
 }
 
 impl TarHeader {
-    pub fn from_bytes(b: &mut [u8]) -> TarHeader {
-        unsafe { ptr::read(b.as_ptr() as *const TarHeader) }
+    pub fn from_bytes(b: &[u8]) -> TarHeader {
+        unsafe { b.as_ptr().cast::<TarHeader>().read() }
+    }
+
+    pub fn has_magic(&self) -> bool {
+        self.magic[..5] == *b"ustar"
     }
 
     pub fn name(&self) -> &str {
-        core::str::from_utf8(&self.name).unwrap()
+        if let Some(len) = self.name.iter().position(|&x| x == 0) {
+            core::str::from_utf8(&self.name[..len])
+        } else {
+            core::str::from_utf8(&self.name)
+        }
+        .unwrap_or("<invalid>")
     }
 
     pub fn size(&self) -> u32 {
@@ -68,28 +77,33 @@
 }
 
 fn parse_octal(text: &[u8]) -> u32 {
-    let mut n: u32 = 0;
-    for x in text.iter() {
-        if *x == (0 as u8) {
-            break;
-        }
-        n = (n << 3) | ((*x as u32) - ('0' as u32));
-    }
-    return n;
+    text.iter()
+        .take_while(|&&x| x != 0)
+        .fold(0u32, |acc, &x| (acc << 3) + (x as u32 - '0' as u32))
 }
 
 pub unsafe fn find_file(name: &str) -> (u32, u32) {
     let mut cursor = EFLASH_START;
-    while cursor < EFLASH_END {
-        let tar_header: *const TarHeader = transmute(cursor);
-        cursor += 512;
-        let tar_name = core::str::from_utf8(&(*tar_header).name).unwrap();
-        if tar_name.contains(name) {
-            let tar_size = parse_octal(&(*tar_header).size);
+    while (cursor + 512) < EFLASH_END {
+        let tar_header = &*(cursor as *const TarHeader);
+        if !tar_header.has_magic() {
+            // Our tar files have the "ustar" magic field; use it
+            // to avoid scanning all EFLASH.
+            break;
+        }
+        cursor += size_of::<TarHeader>() as u32;
+        let tar_name = tar_header.name();
+        let tar_size = tar_header.size();
+        if tar_name == name {
             return (cursor, tar_size);
         } else {
-            let tar_size = parse_octal(&(*tar_header).size);
-            cursor += (tar_size + 511) & !511;
+            fn roundup(a: u32, b: u32) -> u32 {
+                ((a + b - 1) / b) * b
+            }
+            cursor += roundup(
+                tar_size,
+                size_of::<TarHeader>() as u32,
+            );
         }
     }
 
@@ -100,9 +114,14 @@
 // TODO(sleffler): rethink api, e.g. supply a range for bounds check
 pub unsafe fn copy_file(name: &str, offset: u32) -> bool {
     let (cursor, size) = find_file(name);
-    if size == 0 {
-        return false;
+    if cursor != 0 {
+        ptr::copy_nonoverlapping::<u8>(
+            transmute(cursor),
+            transmute(offset),
+            size as usize,
+        );
+        true
+    } else {
+        false
     }
-    ptr::copy_nonoverlapping::<u8>(transmute(cursor), transmute(offset), size as usize);
-    true
 }