From 30e79310abc9ca6929861bf37dc559b27dd5cce5 Mon Sep 17 00:00:00 2001
From: topjohnwu <topjohnwu@gmail.com>
Date: Mon, 14 Apr 2025 13:31:52 -0700
Subject: [PATCH] Make pointers NonNull after error check

---
 native/src/base/dir.rs    |  8 ++------
 native/src/base/result.rs | 39 +++++++++++++++++++++++++++------------
 native/src/base/xwrap.rs  | 20 ++++++++++----------
 3 files changed, 39 insertions(+), 28 deletions(-)

diff --git a/native/src/base/dir.rs b/native/src/base/dir.rs
index 81557c2a3..dad493709 100644
--- a/native/src/base/dir.rs
+++ b/native/src/base/dir.rs
@@ -178,9 +178,7 @@ impl Directory {
     pub fn open(path: &Utf8CStr) -> OsResult<Directory> {
         let dirp = unsafe { libc::opendir(path.as_ptr()) };
         let dirp = dirp.as_os_result("opendir", Some(path), None)?;
-        Ok(Directory {
-            inner: unsafe { NonNull::new_unchecked(dirp) },
-        })
+        Ok(Directory { inner: dirp })
     }
 
     pub fn read(&mut self) -> OsResult<'static, Option<DirEntry>> {
@@ -424,9 +422,7 @@ impl TryFrom<OwnedFd> for Directory {
     fn try_from(fd: OwnedFd) -> OsResult<'static, Self> {
         let dirp = unsafe { libc::fdopendir(fd.into_raw_fd()) };
         let dirp = dirp.as_os_result("fdopendir", None, None)?;
-        Ok(Directory {
-            inner: unsafe { NonNull::new_unchecked(dirp) },
-        })
+        Ok(Directory { inner: dirp })
     }
 }
 
diff --git a/native/src/base/result.rs b/native/src/base/result.rs
index 04b1582ba..50fc5aaec 100644
--- a/native/src/base/result.rs
+++ b/native/src/base/result.rs
@@ -2,6 +2,7 @@ use crate::logging::Formatter;
 use crate::{LogLevel, errno, log_with_args, log_with_formatter};
 use std::fmt::Display;
 use std::panic::Location;
+use std::ptr::NonNull;
 use std::{fmt, io};
 use thiserror::Error;
 
@@ -206,18 +207,21 @@ pub trait LibcReturn
 where
     Self: Copy,
 {
+    type Value;
+
     fn is_error(&self) -> bool;
+    fn map_val(self) -> Self::Value;
 
     fn as_os_result<'a>(
         self,
         name: &'static str,
         arg1: Option<&'a str>,
         arg2: Option<&'a str>,
-    ) -> OsResult<'a, Self> {
+    ) -> OsResult<'a, Self::Value> {
         if self.is_error() {
             Err(OsError::last_os_error(name, arg1, arg2))
         } else {
-            Ok(self)
+            Ok(self.map_val())
         }
     }
 
@@ -227,8 +231,11 @@ where
         arg1: Option<&'a str>,
         arg2: Option<&'a str>,
     ) -> OsResult<'a, ()> {
-        self.as_os_result(name, arg1, arg2)?;
-        Ok(())
+        if self.is_error() {
+            Err(OsError::last_os_error(name, arg1, arg2))
+        } else {
+            Ok(())
+        }
     }
 
     fn check_io_err(self) -> io::Result<()> {
@@ -243,27 +250,35 @@ where
 macro_rules! impl_libc_return {
     ($($t:ty)*) => ($(
         impl LibcReturn for $t {
-            #[inline]
+            type Value = Self;
+
+            #[inline(always)]
             fn is_error(&self) -> bool {
                 *self < 0
             }
+
+            #[inline(always)]
+            fn map_val(self) -> Self::Value {
+                self
+            }
         }
     )*)
 }
 
 impl_libc_return! { i8 i16 i32 i64 isize }
 
-impl<T> LibcReturn for *const T {
-    #[inline]
+impl<T> LibcReturn for *mut T {
+    type Value = NonNull<T>;
+
+    #[inline(always)]
     fn is_error(&self) -> bool {
         self.is_null()
     }
-}
 
-impl<T> LibcReturn for *mut T {
-    #[inline]
-    fn is_error(&self) -> bool {
-        self.is_null()
+    #[inline(always)]
+    fn map_val(self) -> NonNull<T> {
+        // SAFETY: pointer is null checked by is_error
+        unsafe { NonNull::new_unchecked(self.cast()) }
     }
 }
 
diff --git a/native/src/base/xwrap.rs b/native/src/base/xwrap.rs
index e2ab7a0df..8c20d9bb3 100644
--- a/native/src/base/xwrap.rs
+++ b/native/src/base/xwrap.rs
@@ -1,5 +1,10 @@
 // Functions in this file are only for exporting to C++, DO NOT USE IN RUST
 
+use crate::cxx_extern::readlinkat;
+use crate::{
+    CxxResultExt, Directory, FsPath, LibcReturn, Utf8CStr, cstr_buf, slice_from_ptr,
+    slice_from_ptr_mut,
+};
 use libc::{
     c_char, c_uint, c_ulong, c_void, dev_t, mode_t, nfds_t, off_t, pollfd, sockaddr, socklen_t,
 };
@@ -10,12 +15,7 @@ use std::mem::ManuallyDrop;
 use std::os::fd::FromRawFd;
 use std::os::unix::io::RawFd;
 use std::ptr;
-
-use crate::cxx_extern::readlinkat;
-use crate::{
-    CxxResultExt, Directory, FsPath, LibcReturn, Utf8CStr, cstr_buf, slice_from_ptr,
-    slice_from_ptr_mut,
-};
+use std::ptr::NonNull;
 
 fn ptr_to_str<'a>(ptr: *const c_char) -> Option<&'a str> {
     if ptr.is_null() {
@@ -78,7 +78,7 @@ unsafe extern "C" fn xfopen(path: *const c_char, mode: *const c_char) -> *mut li
         libc::fopen(path, mode)
             .as_os_result("fopen", ptr_to_str(path), None)
             .log_cxx()
-            .unwrap_or(ptr::null_mut())
+            .map_or(ptr::null_mut(), NonNull::as_ptr)
     }
 }
 
@@ -88,7 +88,7 @@ unsafe extern "C" fn xfdopen(fd: RawFd, mode: *const c_char) -> *mut libc::FILE
         libc::fdopen(fd, mode)
             .as_os_result("fdopen", None, None)
             .log_cxx()
-            .unwrap_or(ptr::null_mut())
+            .map_or(ptr::null_mut(), NonNull::as_ptr)
     }
 }
 
@@ -175,7 +175,7 @@ unsafe extern "C" fn xopendir(path: *const c_char) -> *mut libc::DIR {
         libc::opendir(path)
             .as_os_result("opendir", ptr_to_str(path), None)
             .log_cxx()
-            .unwrap_or(ptr::null_mut())
+            .map_or(ptr::null_mut(), NonNull::as_ptr)
     }
 }
 
@@ -185,7 +185,7 @@ extern "C" fn xfdopendir(fd: RawFd) -> *mut libc::DIR {
         libc::fdopendir(fd)
             .as_os_result("fdopendir", None, None)
             .log_cxx()
-            .unwrap_or(ptr::null_mut())
+            .map_or(ptr::null_mut(), NonNull::as_ptr)
     }
 }