commit c8338635e7dbc7a90c6ba434ef68c96967ffb12f
author Andrei Homescu <ahomescu@google.com> Fri May 05 03:14:41 2023 +0000
committer Andrei Homescu <ahomescu@google.com> Sat May 06 01:28:45 2023 +0000
tree ddbc026a5286469b1cc739e16dfc6d6d9c7ee130
parent 9e35022143467684ef11ca6c75ddcc3460bce888

libbinder_rs: Use NonNull inside Status

Replacing the raw pointer inside Status with
a NonNull will allow us to wrap Status in an
Option<Status> transparently and transmute between
the two types.

Bug: 278648274
Test: presubmit
Change-Id: I61aea43fdd238d62dfffa6300bc8e0eac8e17a21

libs/binder/rust/src/error.rs

diff --git a/libs/binder/rust/src/error.rs b/libs/binder/rust/src/error.rs
index f6b09ed..ba26062 100644
--- a/libs/binder/rust/src/error.rs
+++ b/libs/binder/rust/src/error.rs
@@ -20,6 +20,7 @@
 use std::error;
 use std::ffi::{CStr, CString};
 use std::fmt::{Debug, Display, Formatter, Result as FmtResult};
+use std::ptr;
 use std::result;
 
 pub use sys::binder_status_t as status_t;
@@ -92,7 +93,7 @@
 /// track of and chain binder errors along with service specific errors.
 ///
 /// Used in AIDL transactions to represent failed transactions.
-pub struct Status(*mut sys::AStatus);
+pub struct Status(ptr::NonNull<sys::AStatus>);
 
 // Safety: The `AStatus` that the `Status` points to must have an entirely thread-safe API for the
 // duration of the `Status` object's lifetime. We ensure this by not allowing mutation of a `Status`
@@ -119,7 +120,7 @@
             // Rust takes ownership of the returned pointer.
             sys::AStatus_newOk()
         };
-        Self(ptr)
+        Self(ptr::NonNull::new(ptr).expect("Unexpected null AStatus pointer"))
     }
 
     /// Create a status object from a service specific error
@@ -147,7 +148,7 @@
                 sys::AStatus_fromServiceSpecificError(err)
             }
         };
-        Self(ptr)
+        Self(ptr::NonNull::new(ptr).expect("Unexpected null AStatus pointer"))
     }
 
     /// Creates a status object from a service specific error.
@@ -161,7 +162,7 @@
             let ptr = unsafe {
                 sys::AStatus_fromExceptionCodeWithMessage(exception as i32, message.as_ptr())
             };
-            Self(ptr)
+            Self(ptr::NonNull::new(ptr).expect("Unexpected null AStatus pointer"))
         } else {
             exception.into()
         }
@@ -181,7 +182,7 @@
     ///
     /// This constructor is safe iff `ptr` is a valid pointer to an `AStatus`.
     pub(crate) unsafe fn from_ptr(ptr: *mut sys::AStatus) -> Self {
-        Self(ptr)
+        Self(ptr::NonNull::new(ptr).expect("Unexpected null AStatus pointer"))
     }
 
     /// Returns `true` if this status represents a successful transaction.
@@ -326,7 +327,7 @@
             // UNKNOWN_ERROR.
             sys::AStatus_fromStatus(status)
         };
-        Self(ptr)
+        Self(ptr::NonNull::new(ptr).expect("Unexpected null AStatus pointer"))
     }
 }
 
@@ -338,7 +339,7 @@
             // Unknown values will be coerced into EX_TRANSACTION_FAILED.
             sys::AStatus_fromExceptionCode(code as i32)
         };
-        Self(ptr)
+        Self(ptr::NonNull::new(ptr).expect("Unexpected null AStatus pointer"))
     }
 }
 
@@ -367,7 +368,7 @@
             // pointee, so we need to delete it here. We know that the pointer
             // will be valid here since `Status` always contains a valid pointer
             // while it is alive.
-            sys::AStatus_delete(self.0);
+            sys::AStatus_delete(self.0.as_mut());
         }
     }
 }
@@ -381,11 +382,15 @@
 /// `Status` object is still alive.
 unsafe impl AsNative<sys::AStatus> for Status {
     fn as_native(&self) -> *const sys::AStatus {
-        self.0
+        self.0.as_ptr()
     }
 
     fn as_native_mut(&mut self) -> *mut sys::AStatus {
-        self.0
+        unsafe {
+            // Safety: The pointer will be valid here since `Status` always
+            // contains a valid and initialized pointer while it is alive.
+            self.0.as_mut()
+        }
     }
 }