fix(axevent)!: Take ownership of returned value in get_string (#149)

`crates/axevent/src/flex.rs`:
- Use custom type instead of `GString` or `GStringPtr` because
  `axevent` supports both keys and values that are not valid UTF-8 and,
  though it seems unlikely that users will want to consume or produce
  such events, supporting it is easy. The biggest drawback is reduced
  ergonomics since users have to convert to normal strings themselves.
- Don't use `mbox` to avoid adding another crate to the public API.
  Furthermore `mbox::MString` requires the string to be valid UTF-8,
  just like `glib::GString` so we would have to use one of the other
  types, and it is not clear to me if or how that would lead to a
  better API.
- Add `debug_assert!(!ptr.is_null())` to make it more likely that
  violations of the preconditions are caught without incurring a cost
  at runtime in production. Since this API is internal to the crate, I
  think the value is low, but so is the cost so might as well have it.

apps-aarch64.checksum

@@ -7,6 +7,6 @@ f47872232e0f2cd4ace9e2e77b07cafb431cbe73  target-aarch64/acap/hello_world_0_0_0_
 75818664787ff2ff7dbdd32fadfe20c23974c867  target-aarch64/acap/inspect_env_0_0_0_aarch64.eap
 9d205dbde1b7144d30fb4c5a45fcaef93cb12810  target-aarch64/acap/licensekey_handler_0_0_0_aarch64.eap
 ed84b4b9ae09256dcef1a7287cd802e39c8b6a92  target-aarch64/acap/reverse_proxy_0_0_0_aarch64.eap
-aee472d1e21f50694fc495c3483ba3dc7bae21e7  target-aarch64/acap/send_event_1_0_0_aarch64.eap
+1451c62ae3306ba77db206db137836561f1069b4  target-aarch64/acap/send_event_1_0_0_aarch64.eap
 e8bfb22e1b1416bff9d33980839ccbc8ce4b9d16  target-aarch64/acap/using_a_build_script_0_0_0_aarch64.eap
 5a533b71d01edb65d15751dd93fa153f5ef343ea  target-aarch64/acap/vapix_access_0_0_0_aarch64.eap

apps-aarch64.filesize

@@ -7,6 +7,6 @@
 2136	target-aarch64/acap/inspect_env_0_0_0_aarch64.eap
 2060	target-aarch64/acap/licensekey_handler_0_0_0_aarch64.eap
 10425	target-aarch64/acap/reverse_proxy_0_0_0_aarch64.eap
-4087	target-aarch64/acap/send_event_1_0_0_aarch64.eap
+4088	target-aarch64/acap/send_event_1_0_0_aarch64.eap
 859	target-aarch64/acap/using_a_build_script_0_0_0_aarch64.eap
 11361	target-aarch64/acap/vapix_access_0_0_0_aarch64.eap

apps/send_event/src/main.rs

@@ -78,16 +78,12 @@ fn main() {
 
 #[cfg(test)]
 mod tests {
-    use std::{
-        ffi::{CStr, CString},
-        time,
-        time::Duration,
-    };
+    use std::{ffi::CStr, time, time::Duration};
 
     use anyhow::bail;
     use axevent::{
         ergo::{date_time, system_time, Declaration, MainLoop, Subscription},
-        flex::{Event, Handler, KeyValueSet},
+        flex::{CStringPtr, Event, Handler, KeyValueSet},
     };
     use log::debug;
 
@@ -102,7 +98,7 @@ mod tests {
         Ok(kvs)
     }
 
-    fn send_and_receive_event(sent: &CStr) -> anyhow::Result<CString> {
+    fn send_and_receive_event(sent: &CStr) -> anyhow::Result<CStringPtr> {
         let main_loop = MainLoop::new();
 
         let handler = Handler::new();
@@ -153,7 +149,13 @@ mod tests {
 
     #[test]
     fn can_send_and_receive_event() {
-        let expected = c"Hello";
+        // axevent supports keys and values that are not valid UTF-8;
+        // using an invalid UTF-8 string is a low-effort way of ensuring that support is propagated.
+        let expected = c"Hello\xc3\x28";
+
+        // Sanity check
+        assert!(expected.to_str().is_err());
+
         let actual = send_and_receive_event(expected).unwrap();
         assert_eq!(actual.as_c_str(), expected);
     }

crates/axevent/src/flex.rs

@@ -13,6 +13,7 @@ use std::{
     ffi::{c_char, c_double, c_int, c_uint, c_void, CStr, CString},
     fmt::Debug,
     process, ptr,
+    ptr::NonNull,
     sync::Mutex,
 };
 
@@ -36,7 +37,7 @@ use glib::{
     translate::{from_glib_full, from_glib_none, IntoGlibPtr},
     DateTime,
 };
-use glib_sys::{gboolean, gpointer, GError};
+use glib_sys::{g_free, gboolean, gpointer, GError};
 use log::debug;
 
 macro_rules! abort_unwind {
@@ -63,6 +64,40 @@ macro_rules! try_func {
     }}
 }
 
+#[repr(transparent)]
+pub struct CStringPtr(NonNull<c_char>);
+
+impl CStringPtr {
+    /// Create an owned string from a foreign allocation
+    ///
+    /// # Safety
+    ///
+    /// In addition to the safety preconditions for [`CStr::from_ptr`] the memory must have been
+    /// allocated in a manner compatible with [`glib_sys::g_free`] and there must be no other
+    /// users of this memory.
+    unsafe fn from_ptr(ptr: *mut c_char) -> Self {
+        debug_assert!(!ptr.is_null());
+        Self(NonNull::new_unchecked(ptr))
+    }
+
+    pub fn as_c_str(&self) -> &CStr {
+        // SAFETY: The preconditions for instantiating this type include all preconditions
+        // for `CStr::from_ptr`.
+        unsafe { CStr::from_ptr(self.0.as_ptr() as *const c_char) }
+    }
+}
+
+impl Drop for CStringPtr {
+    fn drop(&mut self) {
+        // SAFETY: The preconditions for instantiating this type include:
+        // - having full ownership of the memory.
+        // - having allocated the memory in a manner that is compatible with `g_free`.
+        unsafe {
+            g_free(self.0.as_ptr() as *mut c_void);
+        }
+    }
+}
+
 struct Deferred(Option<Box<dyn FnOnce()>>);
 
 impl Drop for Deferred {
@@ -583,7 +618,7 @@ impl KeyValueSet {
         }
     }
 
-    pub fn get_string(&self, key: &CStr, namespace: Option<&CStr>) -> Result<CString> {
+    pub fn get_string(&self, key: &CStr, namespace: Option<&CStr>) -> Result<CStringPtr> {
         unsafe {
             let mut value: *mut c_char = ptr::null_mut();
             try_func!(
@@ -596,7 +631,15 @@ impl KeyValueSet {
                 },
                 &mut value,
             )?;
-            Ok(CString::from(CStr::from_ptr(value)))
+            // SAFETY: This is safe because:
+            // - The foreign function sets the error if the value is null in which case we return
+            //   early above.
+            // - The foreign function creates the value with `g_strdup` so it will be nul
+            //   terminated, reads to up to and including the nul terminator are valid, and it may
+            //   be freed using `g_free`.
+            // - This function owns the memory and does not mutate it.
+            // - Values will never be longer than `isize::MAX` in practice.
+            Ok(CStringPtr::from_ptr(value))
         }
     }