feat: Return a Result<T, PointerError> instead of panic when pass null pointers

Author: jhg

Cargo.toml

@@ -1,6 +1,6 @@
 [package]
 name = "opaque-pointer"
-version = "0.7.2"
+version = "0.8.0"
 description = "Generic functions to work with opaque pointers when use FFI to expose Rust structs"
 authors = ["Jesus Hernandez <jesushdez@protonmail.com>"]
 license = "Unlicense"
@@ -18,8 +18,6 @@ std = []
 alloc = []
 # Allow to use some FFI for C types
 c-types = ["std"]
-# Check pointers before to use it to panic if it is null
-panic-if-null = []
 
 [dependencies]
 log = "^0"

src/c.rs

@@ -6,10 +6,10 @@
 
 use std::ffi::CStr;
 use std::os::raw::c_char;
-use std::str::Utf8Error;
 
-#[cfg(any(feature = "panic-if-null", debug_assertions))]
-use super::panic_if_null;
+use super::null_error_check;
+
+use crate::error::PointerError;
 
 /// Convert a reference to a C string into a static reference to Rust `str`.
 ///
@@ -22,12 +22,11 @@ use super::panic_if_null;
 /// If the C string is not a valid UTF-8 string.
 #[must_use]
 #[inline]
-pub unsafe fn ref_str<'a>(string: *const c_char) -> Result<&'a str, Utf8Error> {
+pub unsafe fn ref_str<'a>(string: *const c_char) -> Result<&'a str, PointerError> {
     // ATTENTION! 'a lifetime is required, does NOT REMOVE it
     // see commit 5a03be91d2da8909986db7c54650f3a7863a91ff fixing 3a1d15f33e8e418ef6bee2b7b9e096780bd2c8ac
-    #[cfg(any(feature = "panic-if-null", debug_assertions))]
-    panic_if_null(string);
+    null_error_check(string)?;
     // CAUTION: this is unsafe
     let string = CStr::from_ptr(string);
-    return string.to_str();
+    return Ok(string.to_str()?);
 }

src/error.rs

@@ -0,0 +1,46 @@
+//! Opaque pointers errors.
+
+#[cfg(all(feature = "std", feature = "c-types"))]
+use std::str::Utf8Error;
+
+/// Errors that can be detected by the functions of this crate.
+///
+/// Of course, invalid address can not be detected, then it's unsafe yet.
+#[derive(Debug)]
+pub enum PointerError {
+    #[allow(missing_docs)] // Obviously, the name is the ref doc.
+    NulPointer,
+    /// Trying to convert to `&str` a C string which content is not valid UTF-8.
+    Utf8Error(Utf8Error),
+}
+
+impl std::fmt::Display for PointerError {
+    fn fmt(&self, f: &mut std::fmt::Formatter) -> std::fmt::Result {
+        match *self {
+            PointerError::NulPointer => {
+                write!(f, "dereference a null pointer will produce a crash")
+            }
+            PointerError::Utf8Error(..) => {
+                write!(f, "the provided C string is not a valid UTF-8 string")
+            }
+        }
+    }
+}
+
+impl std::error::Error for PointerError {
+    fn source(&self) -> Option<&(dyn std::error::Error + 'static)> {
+        match *self {
+            PointerError::NulPointer => None,
+            // The cause is the underlying implementation error type. Is implicitly
+            // cast to the trait object `&error::Error`. This works because the
+            // underlying type already implements the `Error` trait.
+            PointerError::Utf8Error(ref e) => Some(e),
+        }
+    }
+}
+
+impl From<Utf8Error> for PointerError {
+    fn from(err: Utf8Error) -> Self {
+        Self::Utf8Error(err)
+    }
+}

src/lib.rs

@@ -24,13 +24,15 @@ use std::boxed::Box;
 #[cfg(all(feature = "std", feature = "c-types"))]
 pub mod c;
 
-#[cfg(any(feature = "panic-if-null", debug_assertions))]
+pub mod error;
+
 #[inline]
-fn panic_if_null<T>(pointer: *const T) {
+fn null_error_check<T>(pointer: *const T) -> Result<(), crate::error::PointerError> {
     if pointer.is_null() {
-        log::error!("Trying to use a NULL pointer as a opaque pointer to Rust data");
-        unreachable!("Trying to use a NULL pointer as a opaque pointer to Rust data");
+        log::error!("Using a NULL pointer as a opaque pointer to Rust data");
+        return Err(crate::error::PointerError::NulPointer);
     }
+    return Ok(());
 }
 
 /// Get a heap-allocated raw pointer without ownership.
@@ -47,7 +49,7 @@ pub fn raw<T>(data: T) -> *mut T {
 #[cfg(any(feature = "alloc", feature = "std"))]
 #[inline]
 pub unsafe fn free<T>(pointer: *mut T) {
-    own_back(pointer);
+    let _ = own_back(pointer);  // Ignore the must use lint as previous behavior was ignore null pointers
 }
 
 /// Opposite of [`raw<T>()`], to use Rust's ownership as usually.
@@ -60,12 +62,11 @@ pub unsafe fn free<T>(pointer: *mut T) {
 #[doc(alias = "free")]
 #[cfg(any(feature = "alloc", feature = "std"))]
 #[inline]
-pub unsafe fn own_back<T>(pointer: *mut T) -> T {
-    #[cfg(any(feature = "panic-if-null", debug_assertions))]
-    panic_if_null(pointer);
+pub unsafe fn own_back<T>(pointer: *mut T) -> Result<T, crate::error::PointerError> {
+    null_error_check(pointer)?;
     // CAUTION: this is the unsafe part of the function.
     let boxed = Box::from_raw(pointer);
-    return *boxed;
+    return Ok(*boxed);
 }
 
 /// Reference to a object but without back to own it.
@@ -77,11 +78,10 @@ pub unsafe fn own_back<T>(pointer: *mut T) -> T {
 ///
 /// Invalid pointer or call it twice could cause an undefined behavior or heap error and a crash.
 #[inline]
-pub unsafe fn object<'a, T>(pointer: *const T) -> &'a T {
-    #[cfg(any(feature = "panic-if-null", debug_assertions))]
-    panic_if_null(pointer);
+pub unsafe fn object<'a, T>(pointer: *const T) -> Result<&'a T, crate::error::PointerError> {
+    null_error_check(pointer)?;
     // CAUTION: this is unsafe
-    return &*pointer;
+    return Ok(&*pointer);
 }
 
 /// Mutable reference to a object but without back to own it.
@@ -93,9 +93,8 @@ pub unsafe fn object<'a, T>(pointer: *const T) -> &'a T {
 ///
 /// Invalid pointer or call it twice could cause an undefined behavior or heap error and a crash.
 #[inline]
-pub unsafe fn mut_object<'a, T>(pointer: *mut T) -> &'a mut T {
-    #[cfg(any(feature = "panic-if-null", debug_assertions))]
-    panic_if_null(pointer);
+pub unsafe fn mut_object<'a, T>(pointer: *mut T) -> Result<&'a mut T, crate::error::PointerError> {
+    null_error_check(pointer)?;
     // CAUTION: this is unsafe
-    return &mut *pointer;
+    return Ok(&mut *pointer);
 }

tests/pointer.rs

@@ -19,23 +19,23 @@ impl TestIt {
 #[test]
 fn own_back() {
     let pointer = opaque_pointer::raw(TestIt::new(2));
-    let test_it = unsafe { opaque_pointer::own_back(pointer) };
+    let test_it = unsafe { opaque_pointer::own_back(pointer).unwrap() };
     assert_eq!(test_it.get(), 2);
 }
 
 #[test]
 fn immutable_reference() {
     let pointer = opaque_pointer::raw(TestIt::new(2));
-    let object = unsafe { opaque_pointer::object(pointer) };
+    let object = unsafe { opaque_pointer::object(pointer).unwrap() };
     assert_eq!(object.get(), 2);
-    unsafe { opaque_pointer::own_back(pointer) };
+    unsafe { opaque_pointer::own_back(pointer).unwrap() };
 }
 
 #[test]
 fn mutable_reference() {
     let pointer = opaque_pointer::raw(TestIt::new(2));
-    let object = unsafe { opaque_pointer::mut_object(pointer) };
+    let object = unsafe { opaque_pointer::mut_object(pointer).unwrap() };
     object.add(3);
     assert_eq!(object.get(), 5);
-    unsafe { opaque_pointer::own_back(pointer) };
+    unsafe { opaque_pointer::own_back(pointer).unwrap() };
 }