vexos: improve unsafe hygiene in alloc and PAL, fix fs issues

Author: Tropix126

library/std/src/sys/alloc/vexos.rs

@@ -5,7 +5,7 @@ use crate::alloc::{GlobalAlloc, Layout, System};
 use crate::ptr;
 use crate::sync::atomic::{AtomicBool, Ordering};
 
-// symbols defined in the target linkerscript
+// Symbols for heap section boundaries defined in the target's linkerscript
 unsafe extern "C" {
     static mut __heap_start: u8;
     static mut __heap_end: u8;
@@ -21,10 +21,12 @@ unsafe impl dlmalloc::Allocator for Vexos {
         static INIT: AtomicBool = AtomicBool::new(false);
 
         if !INIT.swap(true, Ordering::Relaxed) {
+            // This target has no growable heap, as user memory has a fixed
+            // size/location and VEXos does not manage allocation for us.
             unsafe {
                 (
-                    (&raw mut __heap_start).cast(),
-                    (&raw const __heap_end).byte_offset_from(ptr::addr_of!(__heap_start)) as _,
+                    (&raw mut __heap_start).cast::<u8>(),
+                    (&raw const __heap_end).offset_from_unsigned(&raw const __heap_start),
                     0,
                 )
             }
@@ -63,31 +65,31 @@ unsafe impl GlobalAlloc for System {
     #[inline]
     unsafe fn alloc(&self, layout: Layout) -> *mut u8 {
         // SAFETY: DLMALLOC access is guaranteed to be safe because we are a single-threaded target, which
-        // guarantees unique and non-reentrant access to the allocator.
+        // guarantees unique and non-reentrant access to the allocator. As such, no allocator lock is used.
         // Calling malloc() is safe because preconditions on this function match the trait method preconditions.
         unsafe { DLMALLOC.malloc(layout.size(), layout.align()) }
     }
 
     #[inline]
     unsafe fn alloc_zeroed(&self, layout: Layout) -> *mut u8 {
         // SAFETY: DLMALLOC access is guaranteed to be safe because we are a single-threaded target, which
-        // guarantees unique and non-reentrant access to the allocator.
+        // guarantees unique and non-reentrant access to the allocator. As such, no allocator lock is used.
         // Calling calloc() is safe because preconditions on this function match the trait method preconditions.
         unsafe { DLMALLOC.calloc(layout.size(), layout.align()) }
     }
 
     #[inline]
     unsafe fn dealloc(&self, ptr: *mut u8, layout: Layout) {
         // SAFETY: DLMALLOC access is guaranteed to be safe because we are a single-threaded target, which
-        // guarantees unique and non-reentrant access to the allocator.
+        // guarantees unique and non-reentrant access to the allocator. As such, no allocator lock is used.
         // Calling free() is safe because preconditions on this function match the trait method preconditions.
         unsafe { DLMALLOC.free(ptr, layout.size(), layout.align()) }
     }
 
     #[inline]
     unsafe fn realloc(&self, ptr: *mut u8, layout: Layout, new_size: usize) -> *mut u8 {
         // SAFETY: DLMALLOC access is guaranteed to be safe because we are a single-threaded target, which
-        // guarantees unique and non-reentrant access to the allocator.
+        // guarantees unique and non-reentrant access to the allocator. As such, no allocator lock is used.
         // Calling realloc() is safe because preconditions on this function match the trait method preconditions.
         unsafe { DLMALLOC.realloc(ptr, layout.size(), layout.align(), new_size) }
     }

library/std/src/sys/fs/vexos.rs

@@ -55,7 +55,7 @@ impl FileAttr {
     /// Creates a FileAttr by getting data from an opened file.
     fn from_fd(fd: *mut vex_sdk::FIL) -> io::Result<Self> {
         // `vexFileSize` returns -1 upon error, so u64::try_from will fail on error.
-        if let Some(size) = u64::try_from(unsafe { vex_sdk::vexFileSize(fd) }) {
+        if let Ok(size) = u64::try_from(unsafe { vex_sdk::vexFileSize(fd) }) {
             Ok(Self::File { size })
         } else {
             Err(io::Error::new(io::ErrorKind::InvalidData, "Failed to get file size"))
@@ -64,7 +64,7 @@ impl FileAttr {
 
     fn from_path(path: &Path) -> io::Result<Self> {
         // vexFileStatus returns 3 if the given path is a directory.
-        const FILE_STATUS_DIR: i32 = 3;
+        const FILE_STATUS_DIR: u32 = 3;
 
         run_path_with_cstr(path, &|c_path| {
             let file_type = unsafe { vex_sdk::vexFileStatus(c_path.as_ptr()) };
@@ -84,7 +84,7 @@ impl FileAttr {
 
     pub fn size(&self) -> u64 {
         match self {
-            Self::File { size } => size,
+            Self::File { size } => *size,
             Self::Dir => 0,
         }
     }
@@ -94,7 +94,7 @@ impl FileAttr {
     }
 
     pub fn file_type(&self) -> FileType {
-        self == FileAttr::Dir
+        FileType { is_dir: matches!(self, FileAttr::Dir) }
     }
 
     pub fn modified(&self) -> io::Result<SystemTime> {
@@ -160,7 +160,7 @@ impl DirEntry {
     }
 
     pub fn file_name(&self) -> OsString {
-        self.path.file_name().unwrap_or_default()
+        self.path.file_name().unwrap_or_default().into()
     }
 
     pub fn metadata(&self) -> io::Result<FileAttr> {
@@ -235,14 +235,14 @@ impl File {
                     create,
                     create_new,
                 } => unsafe {
-                    if create_new {
+                    if *create_new {
                         if vex_sdk::vexFileStatus(path.as_ptr()) != 0 {
                             return Err(io::Error::new(
                                 io::ErrorKind::AlreadyExists,
                                 "File exists",
                             ));
                         }
-                    } else if !create {
+                    } else if !*create {
                         if vex_sdk::vexFileStatus(path.as_ptr()) == 0 {
                             return Err(io::Error::new(
                                 io::ErrorKind::NotFound,
@@ -263,14 +263,14 @@ impl File {
                     create,
                     create_new,
                 } => unsafe {
-                    if create_new {
+                    if *create_new {
                         if vex_sdk::vexFileStatus(path.as_ptr()) != 0 {
                             return Err(io::Error::new(
                                 io::ErrorKind::AlreadyExists,
                                 "File exists",
                             ));
                         }
-                    } else if !create {
+                    } else if !*create {
                         if vex_sdk::vexFileStatus(path.as_ptr()) == 0 {
                             return Err(io::Error::new(
                                 io::ErrorKind::NotFound,
@@ -279,7 +279,7 @@ impl File {
                         }
                     }
 
-                    if truncate {
+                    if *truncate {
                         unsafe { vex_sdk::vexFileOpenCreate(path.as_ptr()) }
                     } else {
                         // Open in append, but jump to the start of the file.
@@ -434,7 +434,7 @@ impl File {
                     // we have to calculate the offset from the end of the file ourselves.
                     map_fresult(vex_sdk::vexFileSeek(
                         self.fd.0,
-                        try_convert_offset(self.file_attr()?.size as i64 + offset)?,
+                        try_convert_offset(self.file_attr()?.size() as i64 + offset)?,
                         SEEK_SET,
                     ))?
                 }

library/std/src/sys/pal/vexos/mod.rs

@@ -36,7 +36,7 @@ pub unsafe extern "C" fn _start() -> ! {
     ptr::write_bytes(
         &raw mut __bss_start,
         0,
-        (&raw mut __bss_end).offset_from(&raw mut __bss_start) as usize,
+        (&raw mut __bss_end).offset_from_unsigned(&raw mut __bss_start),
     );
 
     main();