feat: make the notification api safer

src/main.rs

@@ -4,7 +4,7 @@ use std::sync::atomic::{AtomicBool, Ordering};
 use std::sync::{Arc, OnceLock};
 use std::time::Duration;
 
-use hinoirisetr::notify::Notification;
+use hinoirisetr::notify::InitializedNotificationSystem;
 use hinoirisetr::time::Time;
 use hinoirisetr::{Config, apply_settings, compute_settings, debug, error, info, trace, warn};
 use tokio::io::{AsyncBufReadExt, BufReader};
@@ -18,16 +18,17 @@ const SOCKET_PATH: &str = "/tmp/hinoirisetr.sock";
 static CONFIG: OnceLock<Arc<RwLock<Config>>> = OnceLock::new();
 
 enum NotifyState {
-    Enabled(Arc<Notification>),
+    Enabled(InitializedNotificationSystem),
     Disabled,
 }
 
 async fn socket_server(disabled: Arc<AtomicBool>, notify: Arc<Notify>) {
     let listener = UnixListener::bind(SOCKET_PATH).expect("Failed to bind socket");
     trace!("socket server bound");
-    let notification: NotifyState = match unsafe { hinoirisetr::notify::Notification::new() } {
+    let notification: NotifyState =
-        Some(not) => NotifyState::Enabled(Arc::new(not)),
+        match { hinoirisetr::notify::InitializedNotificationSystem::new("hinoirisetr") } {
-        None => {
+            Ok(not) => NotifyState::Enabled(not),
+            Err(_) => {
                 info!("libnotify not found, disabling 'status_notify' command");
                 NotifyState::Disabled
             }
@@ -108,24 +109,15 @@ async fn socket_server(disabled: Arc<AtomicBool>, notify: Arc<Notify>) {
                         NotifyState::Enabled(ref not) => {
                             trace!("notify notification enabled");
                             let timeout = config_handle().read().await.notification_timeout;
-                            unsafe {
+                            match not.show_notification(
-                                not.init("hinoirisetr");
-                                let local_notification =
-                                    not.notify_notification_new("Sunsetting", body.as_str(), "");
-                                not.notify_notification_set_timeout(
-                                    local_notification,
-                                    timeout as i32,
-                                
-                                );
-                                not.notify_notification_show(
-                                    not.notify_notification_new(
                                 "Sunsetting",
-                                        body.as_str(),
+                                &body,
                                 "notification-icon",
-                                    ),
+                                timeout as i32,
-                                    std::ptr::null_mut(),
+                            ) {
-                                );
+                                Ok(_) => {}
-                            }
+                                Err(e) => error!("Failed to show notification: {e:?}"),
+                            };
                         }
                         NotifyState::Disabled => {
                             trace!("notify notification disabled");

src/notify.rs

@@ -1,14 +1,20 @@
-#![allow(clippy::missing_safety_doc)]
+use std::ffi::{CString, NulError, c_char, c_void};
-
+use std::marker::PhantomData;
-use std::ffi::{CString, c_char, c_void};
+use std::sync::Arc;
 
+use libloading::Library;
 use libloading::os::unix::Symbol as UnixSymbol;
-use libloading::{Library, Symbol};
 
-use crate::trace;
+type NotifyInit = unsafe extern "C" fn(*const c_char) -> bool;
+type NotifyUninit = unsafe extern "C" fn();
+type NotifyNotificationNew =
+    unsafe extern "C" fn(*const c_char, *const c_char, *const c_char) -> *mut c_void;
+type NotifyNotificationShow = unsafe extern "C" fn(*mut c_void, *mut *mut c_void) -> i32;
+type NotifyNotificationSetTimeout = unsafe extern "C" fn(*mut c_void, i32);
 
-pub struct Notification {
+/// Represents an uninitialized notification system
-    _lib: Library, // << keep the library alive
+struct NotificationSystem {
+    _lib: Library,
     notify_init: UnixSymbol<NotifyInit>,
     notify_uninit: UnixSymbol<NotifyUninit>,
     notify_notification_new: UnixSymbol<NotifyNotificationNew>,
@@ -16,129 +22,104 @@ pub struct Notification {
     notify_notification_set_timeout: UnixSymbol<NotifyNotificationSetTimeout>,
 }
 
-type NotifyInit = unsafe extern "C" fn(*const i8) -> bool;
+/// Represents an initialized notification system with working notifications
-type NotifyUninit = unsafe extern "C" fn();
+pub struct InitializedNotificationSystem {
+    inner: Arc<NotificationSystem>,
+    _marker: PhantomData<()>,
+}
 
-type NotifyNotificationNew = unsafe extern "C" fn(
+impl NotificationSystem {
-    summary: *const c_char,
+    pub fn new() -> Result<Self, libloading::Error> {
-    body: *const c_char,
-    icon: *const c_char,
-) -> *mut std::os::raw::c_void;
-
-type NotifyNotificationShow = unsafe extern "C" fn(
-    *mut c_void,      // NotifyNotification*
-    *mut *mut c_void, // GError**
-) -> i32;
-
-type NotifyNotificationSetTimeout = unsafe extern "C" fn(
-    notification: *mut std::os::raw::c_void, // NotifyNotification*
-    timeout: i32,                            // gint
-);
-
-impl Notification {
-    /// Try to load `libnotify.so.4` at runtime.
-    pub unsafe fn new() -> Option<Self> {
         unsafe {
-            let (
+            let lib = Library::new("libnotify.so.4")?;
-                lib,
-                notify_init,
-                notify_uninit,
-                notify_notification_new,
-                notify_notification_show,
-                notify_notification_set_timeout,
-            ) = {
-                // 1) Load the library
-                let lib = Library::new("libnotify.so.4").ok()?;
 
-                // 2) Lookup each symbol into its own local
+            Ok(Self {
-                let init_sym: Symbol<NotifyInit> = lib.get(b"notify_init\0").ok()?;
+                notify_init: lib.get::<NotifyInit>(b"notify_init\0")?.into_raw(),
-                let uninit_sym: Symbol<NotifyUninit> = lib.get(b"notify_uninit\0").ok()?;
+                notify_uninit: lib.get::<NotifyUninit>(b"notify_uninit\0")?.into_raw(),
-                let notify_notification_new_sym: Symbol<NotifyNotificationNew> =
+                notify_notification_new: lib
-                    lib.get(b"notify_notification_new\0").ok()?;
+                    .get::<NotifyNotificationNew>(b"notify_notification_new\0")?
-                let notify_notification_show_sym: Symbol<NotifyNotificationShow> =
+                    .into_raw(),
-                    lib.get(b"notify_notification_show\0").ok()?;
+                notify_notification_show: lib
-                let notify_notification_set_timeout_sym: Symbol<NotifyNotificationSetTimeout> =
+                    .get::<NotifyNotificationShow>(b"notify_notification_show\0")?
-                    lib.get(b"notify_notification_set_timeout\0").ok()?;
+                    .into_raw(),
-
+                notify_notification_set_timeout: lib
-                // 3) Transmute their lifetimes to 'static now that we own `lib` forever
+                    .get::<NotifyNotificationSetTimeout>(b"notify_notification_set_timeout\0")?
-                let notify_init = init_sym.into_raw();
+                    .into_raw(),
-                let notify_uninit = uninit_sym.into_raw();
-                let notify_notification_new = notify_notification_new_sym.into_raw();
-                let notify_notification_show = notify_notification_show_sym.into_raw();
-                let notify_notification_set_timeout =
-                    notify_notification_set_timeout_sym.into_raw();
-
-                (
-                    lib,
-                    notify_init,
-                    notify_uninit,
-                    notify_notification_new,
-                    notify_notification_show,
-                    notify_notification_set_timeout,
-                )
-            };
-
-            // 3) Now that the borrows are done, move `lib` and the symbols into your struct
-            Some(Self {
                 _lib: lib,
-                notify_init,
-                notify_uninit,
-                notify_notification_new,
-                notify_notification_show,
-                notify_notification_set_timeout,
             })
         }
     }
 
-    pub unsafe fn init(&self, app_name: &str) -> bool {
+    /// Initialize the notification system. Consumes the uninitialized version
+    /// and returns an initialized version.
+    pub fn init(self, app_name: &str) -> Result<InitializedNotificationSystem, NulError> {
+        let app_name = CString::new(app_name)?;
         unsafe {
-            trace!("Notify::init({app_name:?})");
+            if (self.notify_init)(app_name.as_ptr()) {
-            let cstr = CString::new(app_name).unwrap();
+                Ok(InitializedNotificationSystem {
-            (self.notify_init)(cstr.as_ptr())
+                    inner: Arc::new(self), // Wrap self in Arc
+                    _marker: PhantomData,
+                })
+            } else {
+                panic!("notify_init failed") // Convert to proper error type
+            }
+        }
     }
 }
 
-    pub unsafe fn uninit(&self) {
+impl Drop for NotificationSystem {
-        unsafe {
+    fn drop(&mut self) {
-            trace!("Notify::uninit()");
+        // Safety: Last reference to the library is being dropped
-            (self.notify_uninit)();
+        unsafe { (self.notify_uninit)() };
     }
 }
 
-    pub unsafe fn notify_notification_set_timeout(
+impl InitializedNotificationSystem {
-        &self,
+    pub fn new(app_name: &str) -> Result<Self, Box<dyn std::error::Error>> {
-        notification: *mut std::os::raw::c_void,
+        let lib = NotificationSystem::new()?;
-        timeout: i32,
+        Ok(lib.init(app_name)?)
-    ) {
-        trace!("Notify::notify_notification_set_timeout({notification:?}, {timeout})");
-        unsafe {
-            (self.notify_notification_set_timeout)(notification, timeout);
     }
-    }
+    pub fn show_notification(
-
-    pub unsafe fn notify_notification_new(
         &self,
         summary: &str,
         body: &str,
         icon: &str,
-    ) -> *mut std::os::raw::c_void {
+        timeout_ms: i32,
+    ) -> Result<(), NotificationError> {
+        let summary = CString::new(summary)?;
+        let body = CString::new(body)?;
+        let icon = CString::new(icon)?;
+
         unsafe {
-            trace!("Notify::notify_notification_new({summary:?}, {body:?}, {icon:?})");
+            let notification = (self.inner.notify_notification_new)(
-            let summary = CString::new(summary).unwrap();
+                summary.as_ptr(),
-            let body = CString::new(body).unwrap();
+                body.as_ptr(),
-            let icon = CString::new(icon).unwrap();
+                icon.as_ptr(),
-            (self.notify_notification_new)(summary.as_ptr(), body.as_ptr(), icon.as_ptr())
+            );
+
+            (self.inner.notify_notification_set_timeout)(notification, timeout_ms);
+
+            // Handle potential errors from the C library
+            let mut error_ptr: *mut c_void = std::ptr::null_mut();
+            let success = (self.inner.notify_notification_show)(notification, &mut error_ptr);
+
+            if success != 0 {
+                Ok(())
+            } else {
+                Err(NotificationError::NativeError(error_ptr))
+            }
+        }
     }
 }
 
-    pub unsafe fn notify_notification_show(
+#[derive(Debug)]
-        &self,
+pub enum NotificationError {
-        notification: *mut std::os::raw::c_void,
+    NullByteInString(NulError),
-        error: *mut *mut c_void,
+    NativeError(*mut c_void),
-    ) {
+}
-        unsafe {
+
-            trace!("Notify::notify_notification_show({notification:?}, {error:?})");
+impl From<NulError> for NotificationError {
-            (self.notify_notification_show)(notification, error);
+    fn from(e: NulError) -> Self {
-        }
+        NotificationError::NullByteInString(e)
     }
 }