Better error

libuci-sys/src/lib.rs

@@ -32,7 +32,8 @@ pub use bindings::{
     uci_parse_option, uci_parse_ptr, uci_parse_section, uci_perror, uci_ptr,
     uci_ptr_UCI_LOOKUP_COMPLETE, uci_rename, uci_reorder_section, uci_revert, uci_save,
     uci_section, uci_set, uci_set_backend, uci_set_confdir, uci_set_savedir, uci_type,
-    uci_type_UCI_TYPE_OPTION, uci_type_UCI_TYPE_SECTION, uci_unload, uci_validate_text, UCI_OK,
+    uci_type_UCI_TYPE_OPTION, uci_type_UCI_TYPE_SECTION, uci_unload, uci_validate_text,
+    UCI_ERR_NOTFOUND, UCI_OK,
 };
 
 #[allow(non_upper_case_globals)]

rust-uci/Cargo.toml

@@ -16,3 +16,4 @@ targets = ["x86_64-unknown-linux-gnu"]
 libuci-sys = { version = "^1.1.0", path = "../libuci-sys" }
 log = "^0.4.14"
 libc = "^0.2.91"
+thiserror = "2"

rust-uci/src/error.rs

@@ -2,47 +2,22 @@
 // SPDX-License-Identifier: MIT OR Apache-2.0
 
 use std::ffi::NulError;
-use std::fmt::{Debug, Display, Formatter};
-use std::option::Option::None;
+use std::fmt::Debug;
 use std::str::Utf8Error;
 
-#[derive(Debug, Clone)]
+use thiserror::Error;
+
+#[derive(Debug, Clone, Error, PartialEq)]
 pub enum Error {
+    #[error("{0}")]
     Message(String),
-    Utf8Error(Utf8Error),
-    NulError(NulError),
+    #[error("{0}")]
+    Utf8Error(#[from] Utf8Error),
+    #[error("{0}")]
+    NulError(#[from] NulError),
+    /// uci was unable to find the entry for `entry_identifier`, e.g. during `uci.get()`
+    #[error("Entry not found: {entry_identifier}")]
+    EntryNotFound { entry_identifier: String },
 }
 
 pub type Result<T> = std::result::Result<T, Error>;
-
-impl From<Utf8Error> for Error {
-    fn from(err: Utf8Error) -> Self {
-        Self::Utf8Error(err)
-    }
-}
-
-impl From<NulError> for Error {
-    fn from(err: NulError) -> Self {
-        Self::NulError(err)
-    }
-}
-
-impl std::error::Error for Error {
-    fn source(&self) -> Option<&(dyn std::error::Error + 'static)> {
-        match self {
-            Error::Message(_) => None,
-            Error::Utf8Error(err) => Some(err),
-            Error::NulError(err) => Some(err),
-        }
-    }
-}
-
-impl Display for Error {
-    fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result {
-        match self {
-            Error::Message(msg) => Display::fmt(msg, f),
-            Error::Utf8Error(err) => Display::fmt(err, f),
-            Error::NulError(err) => Display::fmt(err, f),
-        }
-    }
-}

rust-uci/src/lib.rs

@@ -57,6 +57,7 @@ use libuci_sys::{
     uci_type_UCI_TYPE_SECTION, uci_unload,
 };
 use log::debug;
+use std::ffi::c_int;
 use std::sync::Mutex;
 use std::{
     ffi::{CStr, CString},
@@ -65,8 +66,8 @@ use std::{
 
 use crate::error::{Error, Result};
 
-#[allow(clippy::cast_possible_wrap)]
-const UCI_OK: i32 = libuci_sys::UCI_OK as i32;
+const UCI_OK: c_int = libuci_sys::UCI_OK as c_int;
+const UCI_ERR_NOTFOUND: c_int = libuci_sys::UCI_ERR_NOTFOUND as c_int;
 
 // Global lock to ensure that only one instance of libuci function calls is running at the time.
 // Necessary because libuci uses thread-unsafe functions with global state (e.g., strtok).
@@ -140,7 +141,7 @@ impl DerefMut for UciPtr {
 
 impl Drop for UciPtr {
     fn drop(&mut self) {
-        unsafe { CString::from_raw(self.1) };
+        drop(unsafe { CString::from_raw(self.1) });
     }
 }
 
@@ -216,7 +217,8 @@ impl Uci {
     ///
     /// Allowed keys are like `network.wan.proto`, `network.@interface[-1].iface`, `network.wan` and `network.@interface[-1]`
     ///
-    /// if the deletion failed an `Err` is returned.
+    /// If the deletion failed an `Err` is returned.
+    /// If the entry does not exist before deletion, an `Err` with [`Error::EntryNotFound`] is returned.
     pub fn delete(&mut self, identifier: &str) -> Result<()> {
         let mut ptr = self.get_ptr(identifier)?;
         libuci_locked!(self, {
@@ -231,16 +233,18 @@ impl Uci {
                 )));
             }
             let result = unsafe { uci_save(self.ctx, ptr.p) };
-            if result == UCI_OK {
-                Ok(())
-            } else {
-                Err(Error::Message(format!(
+            match result {
+                UCI_OK => Ok(()),
+                UCI_ERR_NOTFOUND => Err(Error::EntryNotFound {
+                    entry_identifier: identifier.to_string(),
+                }),
+                _ => Err(Error::Message(format!(
                     "Could not save uci key: {}, {}, {}",
                     identifier,
                     result,
                     self.get_last_error()
                         .unwrap_or_else(|_| String::from("Unknown"))
-                )))
+                ))),
             }
         })
     }
@@ -355,11 +359,13 @@ impl Uci {
     ///
     /// Allowed keys are like `network.wan.proto`, `network.@interface[-1].iface`, `network.lan` and `network.@interface[-1]`
     ///
-    /// if the entry does not exist an `Err` is returned.
+    /// If the entry does not exist an `Err` with [`Error::EntryNotFound`] is returned.
     pub fn get(&mut self, key: &str) -> Result<String> {
         let ptr = libuci_locked!(self, { self.get_ptr(key)? });
         if ptr.flags & uci_ptr_UCI_LOOKUP_COMPLETE == 0 {
-            return Err(Error::Message(format!("Lookup failed: {}", key)));
+            return Err(Error::EntryNotFound {
+                entry_identifier: key.into(),
+            });
         }
         let last = unsafe { *ptr.last };
         #[allow(non_upper_case_globals)]
@@ -407,7 +413,7 @@ impl Uci {
                 );
                 Ok(String::from(typ))
             }
-            _ => return Err(Error::Message(format!("unsupported type: {}", last.type_))),
+            _ => Err(Error::Message(format!("unsupported type: {}", last.type_))),
         }
     }
 
@@ -436,14 +442,22 @@ impl Uci {
         let raw = libuci_locked!(self, {
             let raw = CString::new(identifier)?.into_raw();
             let result = unsafe { uci_lookup_ptr(self.ctx, &mut ptr, raw, true) };
-            if result != UCI_OK {
-                return Err(Error::Message(format!(
-                    "Could not parse uci key: {}, {}, {}",
-                    identifier,
-                    result,
-                    self.get_last_error()
-                        .unwrap_or_else(|_| String::from("Unknown"))
-                )));
+            match result {
+                UCI_OK => (),
+                UCI_ERR_NOTFOUND => {
+                    return Err(Error::EntryNotFound {
+                        entry_identifier: identifier.to_string(),
+                    });
+                }
+                _ => {
+                    return Err(Error::Message(format!(
+                        "Could not parse uci key: {}, {}, {}",
+                        identifier,
+                        result,
+                        self.get_last_error()
+                            .unwrap_or_else(|_| String::from("Unknown"))
+                    )));
+                }
             }
             raw
         });