From 6e05258d368aa3bbe2ded88f4b49cd0234b72618 Mon Sep 17 00:00:00 2001
From: PThorpe92 <preston@unlockedlabs.org>
Date: Sun, 12 Jan 2025 15:48:26 -0500
Subject: [PATCH] Add safety comments and clean up extension types

---
 core/types.rs              | 12 ++++--------
 extensions/uuid/src/lib.rs | 11 ++++++-----
 limbo_extension/src/lib.rs | 29 ++++++++++++++++++-----------
 3 files changed, 28 insertions(+), 24 deletions(-)

diff --git a/core/types.rs b/core/types.rs
index 92d02fce5..3d84da194 100644
--- a/core/types.rs
+++ b/core/types.rs
@@ -123,14 +123,10 @@ impl OwnedValue {
                 OwnedValue::Float(float)
             }
             ExtValueType::Text => {
-                if v.value.is_null() {
-                    OwnedValue::Null
-                } else {
-                    let Some(text) = ExtTextValue::from_value(v) else {
-                        return OwnedValue::Null;
-                    };
-                    OwnedValue::build_text(std::rc::Rc::new(unsafe { text.as_str().to_string() }))
-                }
+                let Some(text) = (unsafe { ExtTextValue::from_value(v) }) else {
+                    return OwnedValue::Null;
+                };
+                OwnedValue::build_text(std::rc::Rc::new(unsafe { text.as_str().to_string() }))
             }
             ExtValueType::Blob => {
                 let blob_ptr = v.value as *mut ExtBlob;
diff --git a/extensions/uuid/src/lib.rs b/extensions/uuid/src/lib.rs
index 7380f82d3..df8581955 100644
--- a/extensions/uuid/src/lib.rs
+++ b/extensions/uuid/src/lib.rs
@@ -73,7 +73,9 @@ declare_scalar_functions! {
                 Value::from_integer(unix as i64)
             }
             ValueType::Text => {
-                let text = TextValue::from_value(&args[0]).unwrap();
+                let Some(text) = (unsafe {TextValue::from_value(&args[0])}) else {
+                    return Value::null();
+                };
                 let uuid = uuid::Uuid::parse_str(unsafe {text.as_str()}).unwrap();
                 let unix = uuid_to_unix(uuid.as_bytes());
                 Value::from_integer(unix as i64)
@@ -106,16 +108,15 @@ declare_scalar_functions! {
             log::debug!("uuid_blob was passed a non-text arg");
             return Value::null();
         }
-        if let Some(text) = TextValue::from_value(&args[0]) {
+        let Some(text) = (unsafe { TextValue::from_value(&args[0])}) else {
+            return Value::null();
+        };
         match uuid::Uuid::parse_str(unsafe {text.as_str()}) {
             Ok(uuid) => {
                     Value::from_blob(uuid.as_bytes().to_vec())
                 }
             Err(_) => Value::null()
         }
-        } else {
-            Value::null()
-        }
     }
 }
 
diff --git a/limbo_extension/src/lib.rs b/limbo_extension/src/lib.rs
index 7bfc881e5..f8eb19dcf 100644
--- a/limbo_extension/src/lib.rs
+++ b/limbo_extension/src/lib.rs
@@ -137,8 +137,8 @@ impl std::fmt::Debug for Value {
 
 #[repr(C)]
 pub struct TextValue {
-    pub text: *const u8,
-    pub len: u32,
+    text: *const u8,
+    len: u32,
 }
 
 impl std::fmt::Debug for TextValue {
@@ -168,22 +168,27 @@ impl TextValue {
         }
     }
 
-    pub fn from_value(value: &Value) -> Option<&Self> {
+    /// # Safety
+    /// Safe to call if the pointer is null, returns None
+    /// if the value is not a text type or if the value is null
+    pub unsafe fn from_value(value: &Value) -> Option<&Self> {
         if value.value_type != ValueType::Text {
             return None;
         }
-        unsafe { Some(&*(value.value as *const TextValue)) }
+        if value.value.is_null() {
+            return None;
+        }
+        Some(&*(value.value as *const TextValue))
     }
 
     /// # Safety
-    /// The caller must ensure that the text is a valid UTF-8 string
+    /// If self.text is null we safely return an empty string but
+    /// the caller must ensure that the underlying value is valid utf8
     pub unsafe fn as_str(&self) -> &str {
         if self.text.is_null() {
             return "";
         }
-        unsafe {
-            std::str::from_utf8_unchecked(std::slice::from_raw_parts(self.text, self.len as usize))
-        }
+        std::str::from_utf8_unchecked(std::slice::from_raw_parts(self.text, self.len as usize))
     }
 }
 
@@ -258,7 +263,11 @@ impl Value {
         }
     }
 
-    pub unsafe fn free(&mut self) {
+    /// # Safety
+    /// consumes the value while freeing the underlying memory with null check.
+    /// however this does assume that the type was properly constructed with
+    /// the appropriate value_type and value.
+    pub unsafe fn free(self) {
         if self.value.is_null() {
             return;
         }
@@ -277,7 +286,5 @@ impl Value {
             }
             ValueType::Null => {}
         }
-
-        self.value = std::ptr::null_mut();
     }
 }