From 56961fe6759fafb8c8e50d38b6da5f4816f83d2a Mon Sep 17 00:00:00 2001
From: "Park, Sanguk" <noorim.ailen@gmail.com>
Date: Thu, 12 Sep 2019 01:37:24 +0900
Subject: [PATCH] Resolve simple issues on the review of #52

---
 hfo2/src/api.rs         |  4 +++-
 hfo2/src/fdt.rs         | 10 +++++-----
 hfo2/src/fdt_handler.rs | 30 ++++++++++++------------------
 hfo2/src/mm.rs          |  5 +++--
 4 files changed, 23 insertions(+), 26 deletions(-)

diff --git a/hfo2/src/api.rs b/hfo2/src/api.rs
index ca5871319..4ddf4813f 100644
--- a/hfo2/src/api.rs
+++ b/hfo2/src/api.rs
@@ -219,7 +219,7 @@ pub unsafe extern "C" fn api_vcpu_get_count(
 
     let vm = some_or!(hafnium().vm_manager.get(vm_id), return 0);
 
-    vm.vcpus.len() as spci_vcpu_count_t
+    vm.vcpus.len() as _
 }
 
 /// This function is called by the architecture-specific context switching
@@ -584,6 +584,8 @@ pub unsafe extern "C" fn api_spci_msg_send(
         // at spci_msg_handle_architected_message will make several accesses to
         // fields in message_buffer. The memory area message_buffer must be
         // exclusively owned by Hf so that TOCTOU issues do not arise.
+        // TODO(HfO2): Refactor `spci_*` functions, in order to pass references 
+        // to VmInner.
         let ret = spci_msg_handle_architected_message(
             &ManuallyDrop::new(VmLocked::from_raw(to as *const _ as usize as *mut _)),
             &ManuallyDrop::new(VmLocked::from_raw(from)),
diff --git a/hfo2/src/fdt.rs b/hfo2/src/fdt.rs
index 969951610..ea42cf4bc 100644
--- a/hfo2/src/fdt.rs
+++ b/hfo2/src/fdt.rs
@@ -106,6 +106,10 @@ impl FdtTokenizer {
         self.cur = round_up(self.cur as usize, FDT_TOKEN_ALIGNMENT) as _;
     }
 
+    unsafe fn iter(&self) -> impl Iterator<Item = *const u8> {
+        slice::from_raw_parts(self.cur, self.end.offset_from(self.cur) as usize).iter().map(|p| p as *const u8)
+    }
+
     unsafe fn u32(&mut self) -> Option<u32> {
         let next = self.cur.add(mem::size_of::<u32>());
         if next > self.end {
@@ -143,9 +147,7 @@ impl FdtTokenizer {
     }
 
     unsafe fn str(&mut self) -> Option<*const u8> {
-        let mut p = self.cur;
-
-        while p < self.end {
+        for p in self.iter() {
             if *p == 0 {
                 // Found the end of the string.
                 let res = self.cur;
@@ -153,8 +155,6 @@ impl FdtTokenizer {
                 self.align();
                 return Some(res);
             }
-
-            p = p.add(1);
         }
 
         None
diff --git a/hfo2/src/fdt_handler.rs b/hfo2/src/fdt_handler.rs
index 3e6cc3c5c..a2636f7c4 100644
--- a/hfo2/src/fdt_handler.rs
+++ b/hfo2/src/fdt_handler.rs
@@ -126,16 +126,13 @@ impl FdtNode {
                 }
             }
 
-            let (mut data, mut size) =
-                if let Ok((data, size)) = node.read_property("reg\0".as_ptr()) {
-                    (data, size)
+            let (mut data, mut size) = ok_or!(node.read_property("reg\0".as_ptr()), {
+                if node.next_sibling().is_none() {
+                    break;
                 } else {
-                    if node.next_sibling().is_none() {
-                        break;
-                    } else {
-                        continue;
-                    }
-                };
+                    continue;
+                }
+            });
 
             // Get all entries for this CPU.
             while size as usize >= address_size {
@@ -200,16 +197,13 @@ impl FdtNode {
                     continue;
                 }
             }
-            let (mut data, mut size) =
-                if let Ok((data, size)) = node.read_property("reg\0".as_ptr()) {
-                    (data, size)
+            let (mut data, mut size) = ok_or!(node.read_property("reg\0".as_ptr()), {
+                if node.next_sibling().is_none() {
+                    break;
                 } else {
-                    if node.next_sibling().is_none() {
-                        break;
-                    } else {
-                        continue;
-                    }
-                };
+                    continue;
+                }
+            });
 
             // Traverse all memory ranges within this node.
             while size as usize >= entry_size {
diff --git a/hfo2/src/mm.rs b/hfo2/src/mm.rs
index 114f8f213..63f6f27cc 100644
--- a/hfo2/src/mm.rs
+++ b/hfo2/src/mm.rs
@@ -1238,8 +1238,9 @@ pub unsafe extern "C" fn mm_unmap(
     stage1_locked.unmap(begin, end, mpool).is_ok()
 }
 
-/// This function is only used in one unit test (fdt/find_memory_ranges.)
-/// Unsafety doesn't really matter.
+/// TODO(HfO2): This function is only used in one unit test
+/// (fdt/find_memory_ranges.) Unsafety doesn't really matter. Resolve #46, then
+/// we can remove this.
 #[no_mangle]
 pub unsafe extern "C" fn mm_init(mpool: *const MPool) -> bool {
     let mm = some_or!(MemoryManager::new(&*mpool), return false);