From 37481317b07a34a25e2ef58f64d79e0d5fa9c690 Mon Sep 17 00:00:00 2001 From: Johann Carl Meyer Date: Wed, 12 Nov 2025 14:08:08 +0100 Subject: [PATCH 1/4] more explicit error makes entry not found error explicit in the type system --- libuci-sys/src/lib.rs | 3 ++- rust-uci/Cargo.toml | 1 + rust-uci/src/error.rs | 49 +++++++++++-------------------------------- rust-uci/src/lib.rs | 46 +++++++++++++++++++++++++--------------- 4 files changed, 44 insertions(+), 55 deletions(-) diff --git a/libuci-sys/src/lib.rs b/libuci-sys/src/lib.rs index 1894820..eb0acaf 100644 --- a/libuci-sys/src/lib.rs +++ b/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)] diff --git a/rust-uci/Cargo.toml b/rust-uci/Cargo.toml index 5772d92..f381fcf 100644 --- a/rust-uci/Cargo.toml +++ b/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" diff --git a/rust-uci/src/error.rs b/rust-uci/src/error.rs index b223115..a2b0868 100644 --- a/rust-uci/src/error.rs +++ b/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)] 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_identifyer`, e.g. during `uci.get()` + #[error("Entry not found: {entry_identifyer}")] + EntryNotFound { entry_identifyer: String }, } pub type Result = std::result::Result; - -impl From for Error { - fn from(err: Utf8Error) -> Self { - Self::Utf8Error(err) - } -} - -impl From 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), - } - } -} diff --git a/rust-uci/src/lib.rs b/rust-uci/src/lib.rs index 16f72de..571e850 100644 --- a/rust-uci/src/lib.rs +++ b/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). @@ -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_identifyer: 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,7 +359,7 @@ 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 { let ptr = libuci_locked!(self, { self.get_ptr(key)? }); if ptr.flags & uci_ptr_UCI_LOOKUP_COMPLETE == 0 { @@ -436,14 +440,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_identifyer: 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 }); From 84f84d3c7fa8a5c89a1472afc3b0551a7d17a9e3 Mon Sep 17 00:00:00 2001 From: Johann Carl Meyer Date: Wed, 12 Nov 2025 14:13:50 +0100 Subject: [PATCH 2/4] fix warnings and clippy lints --- rust-uci/src/lib.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/rust-uci/src/lib.rs b/rust-uci/src/lib.rs index 571e850..ff8c818 100644 --- a/rust-uci/src/lib.rs +++ b/rust-uci/src/lib.rs @@ -141,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) }); } } @@ -411,7 +411,7 @@ impl Uci { ); Ok(String::from(typ)) } - _ => return Err(Error::Message(format!("unsupported type: {}", last.type_))), + _ => Err(Error::Message(format!("unsupported type: {}", last.type_))), } } From 000081ca7753f0caa1af75c19eb1fb00ee963e19 Mon Sep 17 00:00:00 2001 From: Johann Carl Meyer Date: Thu, 13 Nov 2025 09:25:03 +0100 Subject: [PATCH 3/4] derive PartialEq and fix typo --- rust-uci/src/error.rs | 8 ++++---- rust-uci/src/lib.rs | 4 ++-- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/rust-uci/src/error.rs b/rust-uci/src/error.rs index a2b0868..4f335e6 100644 --- a/rust-uci/src/error.rs +++ b/rust-uci/src/error.rs @@ -7,7 +7,7 @@ use std::str::Utf8Error; use thiserror::Error; -#[derive(Debug, Clone, Error)] +#[derive(Debug, Clone, Error, PartialEq)] pub enum Error { #[error("{0}")] Message(String), @@ -15,9 +15,9 @@ pub enum Error { Utf8Error(#[from] Utf8Error), #[error("{0}")] NulError(#[from] NulError), - /// uci was unable to find the entry for `entry_identifyer`, e.g. during `uci.get()` - #[error("Entry not found: {entry_identifyer}")] - EntryNotFound { entry_identifyer: String }, + /// 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 = std::result::Result; diff --git a/rust-uci/src/lib.rs b/rust-uci/src/lib.rs index ff8c818..9e76092 100644 --- a/rust-uci/src/lib.rs +++ b/rust-uci/src/lib.rs @@ -236,7 +236,7 @@ impl Uci { match result { UCI_OK => Ok(()), UCI_ERR_NOTFOUND => Err(Error::EntryNotFound { - entry_identifyer: identifier.to_string(), + entry_identifier: identifier.to_string(), }), _ => Err(Error::Message(format!( "Could not save uci key: {}, {}, {}", @@ -444,7 +444,7 @@ impl Uci { UCI_OK => (), UCI_ERR_NOTFOUND => { return Err(Error::EntryNotFound { - entry_identifyer: identifier.to_string(), + entry_identifier: identifier.to_string(), }); } _ => { From 43b4173189920af22e084d0e8323d535ada4d530 Mon Sep 17 00:00:00 2001 From: Johann Carl Meyer Date: Thu, 13 Nov 2025 09:47:17 +0100 Subject: [PATCH 4/4] add EntryNotFound error return --- rust-uci/src/lib.rs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/rust-uci/src/lib.rs b/rust-uci/src/lib.rs index 9e76092..83006f4 100644 --- a/rust-uci/src/lib.rs +++ b/rust-uci/src/lib.rs @@ -363,7 +363,9 @@ impl Uci { pub fn get(&mut self, key: &str) -> Result { 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)]