Swss common exceptions

crates/swss-common/src/lib.rs

@@ -4,17 +4,16 @@ mod bindings {
 }
 mod types;
 
-use bindings::*;
 pub use types::*;
 
 /// Rust wrapper around `swss::SonicDBConfig::initialize`.
-pub fn sonic_db_config_initialize(path: &str) {
+pub fn sonic_db_config_initialize(path: &str) -> Result<(), Exception> {
     let path = cstr(path);
-    unsafe { bindings::SWSSSonicDBConfig_initialize(path.as_ptr()) }
+    unsafe { Exception::try0(bindings::SWSSSonicDBConfig_initialize(path.as_ptr())) }
 }
 
 /// Rust wrapper around `swss::SonicDBConfig::initializeGlobalConfig`.
-pub fn sonic_db_config_initialize_global(path: &str) {
+pub fn sonic_db_config_initialize_global(path: &str) -> Result<(), Exception> {
     let path = cstr(path);
-    unsafe { bindings::SWSSSonicDBConfig_initializeGlobalConfig(path.as_ptr()) }
+    unsafe { Exception::try0(bindings::SWSSSonicDBConfig_initializeGlobalConfig(path.as_ptr())) }
 }

crates/swss-common/src/types.rs

@@ -6,6 +6,7 @@ mod cxxstring;
 mod dbconnector;
 mod producerstatetable;
 mod subscriberstatetable;
+mod table;
 mod zmqclient;
 mod zmqconsumerstatetable;
 mod zmqproducerstatetable;
@@ -16,31 +17,100 @@ pub use cxxstring::{CxxStr, CxxString};
 pub use dbconnector::{DbConnectionInfo, DbConnector};
 pub use producerstatetable::ProducerStateTable;
 pub use subscriberstatetable::SubscriberStateTable;
+pub use table::Table;
 pub use zmqclient::ZmqClient;
 pub use zmqconsumerstatetable::ZmqConsumerStateTable;
 pub use zmqproducerstatetable::ZmqProducerStateTable;
 pub use zmqserver::ZmqServer;
 
-use crate::*;
+use crate::bindings::*;
 use cxxstring::RawMutableSWSSString;
 use std::{
     any::Any,
     collections::HashMap,
     error::Error,
     ffi::{CStr, CString},
     fmt::Display,
+    mem::MaybeUninit,
     slice,
     str::FromStr,
 };
 
 pub(crate) fn cstr(s: impl AsRef<[u8]>) -> CString {
-    CString::new(s.as_ref()).unwrap()
+    CString::new(s.as_ref()).expect("Bytes being converted to a C string already contains a null byte")
 }
 
-pub(crate) unsafe fn str(p: *const i8) -> String {
-    CStr::from_ptr(p).to_str().unwrap().to_string()
+/// Take a malloc'd c string and convert it to a native String
+pub(crate) unsafe fn take_cstr(p: *const i8) -> String {
+    let s = CStr::from_ptr(p)
+        .to_str()
+        .expect("C string being converted to Rust String contains invalid UTF-8")
+        .to_string();
+    libc::free(p as *mut libc::c_void);
+    s
 }
 
+pub type Result<T, E = Exception> = std::result::Result<T, E>;
+
+/// Rust version of a failed `SWSSResult`.
+///
+/// When an `SWSSResult` is success/`SWSSException_None`, the rust function will return `Ok(..)`.
+/// Otherwise, the rust function will return `Err(Exception)`
+#[derive(Debug, Clone)]
+pub struct Exception {
+    message: String,
+    location: String,
+}
+
+impl Exception {
+    pub(crate) unsafe fn take_raw(res: &mut SWSSResult) -> Self {
+        Self {
+            message: CxxString::take_raw(&mut res.message)
+                .expect("SWSSResult missing message")
+                .to_string_lossy()
+                .into_owned(),
+            location: CxxString::take_raw(&mut res.location)
+                .expect("SWSSResult missing location")
+                .to_string_lossy()
+                .into_owned(),
+        }
+    }
+
+    /// Call an SWSS function that takes one output pointer `*mut T` and returns an `SWSSResult`, and transform that into `Result<T, Exception>`.
+    pub(crate) unsafe fn try1<T, F: FnOnce(*mut T) -> SWSSResult>(f: F) -> Result<T, Exception> {
+        let mut t: MaybeUninit<T> = MaybeUninit::uninit();
+        let mut result: SWSSResult = f(t.as_mut_ptr());
+        if result.exception == SWSSException_SWSSException_None {
+            Ok(t.assume_init())
+        } else {
+            Err(Exception::take_raw(&mut result))
+        }
+    }
+
+    /// Transform an `SWSSResult` into `Result<(), Exception>`, for SWSS functions that take no output pointer.
+    pub(crate) unsafe fn try0(res: SWSSResult) -> Result<(), Exception> {
+        Exception::try1(|_| res)
+    }
+
+    /// Get an informational string about the error that occurred.
+    pub fn message(&self) -> &str {
+        &self.message
+    }
+
+    /// Get an informational string about the where in the code the error occurred.
+    pub fn location(&self) -> &str {
+        &self.location
+    }
+}
+
+impl Display for Exception {
+    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
+        write!(f, "[{}] {}", self.location, self.message)
+    }
+}
+
+impl Error for Exception {}
+
 /// Rust version of the return type from `swss::Select::select`.
 ///
 /// This enum does not include the `swss::Select::ERROR` because errors are handled via a different
@@ -130,12 +200,15 @@ impl Display for InvalidKeyOperationString {
 
 impl Error for InvalidKeyOperationString {}
 
+/// Rust version of `vector<swss::FieldValueTuple>`.
+pub type FieldValues = HashMap<String, CxxString>;
+
 /// Rust version of `swss::KeyOpFieldsValuesTuple`.
 #[derive(Debug, Clone, PartialEq, Eq)]
 pub struct KeyOpFieldValues {
     pub key: String,
     pub operation: KeyOperation,
-    pub field_values: HashMap<String, CxxString>,
+    pub field_values: FieldValues,
 }
 
 /// Intended for testing, ordered by key.
@@ -153,12 +226,12 @@ impl Ord for KeyOpFieldValues {
 }
 
 /// Takes ownership of an `SWSSFieldValueArray` and turns it into a native representation.
-pub(crate) unsafe fn take_field_value_array(arr: SWSSFieldValueArray) -> HashMap<String, CxxString> {
+pub(crate) unsafe fn take_field_value_array(arr: SWSSFieldValueArray) -> FieldValues {
     let mut out = HashMap::with_capacity(arr.len as usize);
     if !arr.data.is_null() {
         let entries = slice::from_raw_parts_mut(arr.data, arr.len as usize);
         for fv in entries {
-            let field = str(fv.field);
+            let field = take_cstr(fv.field);
             let value = CxxString::take_raw(&mut fv.value).unwrap();
             out.insert(field, value);
         }
@@ -174,7 +247,7 @@ pub(crate) unsafe fn take_key_op_field_values_array(kfvs: SWSSKeyOpFieldValuesAr
         unsafe {
             let entries = slice::from_raw_parts_mut(kfvs.data, kfvs.len as usize);
             for kfv in entries {
-                let key = str(kfv.key);
+                let key = take_cstr(kfv.key);
                 let operation = KeyOperation::from_raw(kfv.operation);
                 let field_values = take_field_value_array(kfv.fieldValues);
                 out.push(KeyOpFieldValues {
@@ -189,6 +262,18 @@ pub(crate) unsafe fn take_key_op_field_values_array(kfvs: SWSSKeyOpFieldValuesAr
     out
 }
 
+/// Takes ownership of an `SWSSStringArray` and turns it into a native representation.
+pub(crate) unsafe fn take_string_array(arr: SWSSStringArray) -> Vec<String> {
+    let out = if !arr.data.is_null() {
+        let entries = slice::from_raw_parts(arr.data, arr.len as usize);
+        Vec::from_iter(entries.iter().map(|&s| take_cstr(s)))
+    } else {
+        Vec::new()
+    };
+    SWSSStringArray_free(arr);
+    out
+}
+
 pub(crate) fn make_field_value_array<I, F, V>(fvs: I) -> (SWSSFieldValueArray, KeepAlive)
 where
     I: IntoIterator<Item = (F, V)>,

crates/swss-common/src/types/async_util.rs

@@ -35,10 +35,10 @@ macro_rules! impl_read_data_async {
         pub async fn read_data_async(&mut self) -> ::std::io::Result<()> {
             use ::tokio::io::{unix::AsyncFd, Interest};
 
-            let _ready_guard = AsyncFd::with_interest(self.get_fd(), Interest::READABLE)?
-                .readable()
-                .await?;
-            self.read_data(Duration::from_secs(0), false);
+            let fd = self.get_fd().map_err(::std::io::Error::other)?;
+            let _ready_guard = AsyncFd::with_interest(fd, Interest::READABLE)?.readable().await?;
+            self.read_data(Duration::from_secs(0), false)
+                .map_err(::std::io::Error::other)?;
             Ok(())
         }
     };

crates/swss-common/src/types/consumerstatetable.rs

@@ -1,5 +1,5 @@
 use super::*;
-use crate::*;
+use crate::bindings::*;
 use std::{os::fd::BorrowedFd, ptr::null, time::Duration};
 
 /// Rust wrapper around `swss::ConsumerStateTable`.
@@ -10,39 +10,47 @@ pub struct ConsumerStateTable {
 }
 
 impl ConsumerStateTable {
-    pub fn new(db: DbConnector, table_name: &str, pop_batch_size: Option<i32>, pri: Option<i32>) -> Self {
+    pub fn new(db: DbConnector, table_name: &str, pop_batch_size: Option<i32>, pri: Option<i32>) -> Result<Self> {
         let table_name = cstr(table_name);
         let pop_batch_size = pop_batch_size.as_ref().map(|n| n as *const i32).unwrap_or(null());
         let pri = pri.as_ref().map(|n| n as *const i32).unwrap_or(null());
-        let ptr = unsafe { SWSSConsumerStateTable_new(db.ptr, table_name.as_ptr(), pop_batch_size, pri) };
-        Self { ptr, _db: db }
+        let ptr = unsafe {
+            Exception::try1(|p_cst| SWSSConsumerStateTable_new(db.ptr, table_name.as_ptr(), pop_batch_size, pri, p_cst))
+        }?;
+        Ok(Self { ptr, _db: db })
     }
 
-    pub fn pops(&self) -> Vec<KeyOpFieldValues> {
+    pub fn pops(&self) -> Result<Vec<KeyOpFieldValues>> {
         unsafe {
-            let ans = SWSSConsumerStateTable_pops(self.ptr);
-            take_key_op_field_values_array(ans)
+            let arr = Exception::try1(|p_arr| SWSSConsumerStateTable_pops(self.ptr, p_arr))?;
+            Ok(take_key_op_field_values_array(arr))
         }
     }
 
-    pub fn get_fd(&self) -> BorrowedFd {
-        let fd = unsafe { SWSSConsumerStateTable_getFd(self.ptr) };
-
+    pub fn get_fd(&self) -> Result<BorrowedFd> {
         // SAFETY: This fd represents the underlying redis connection, which should stay alive
         // as long as the DbConnector does.
-        unsafe { BorrowedFd::borrow_raw(fd.try_into().unwrap()) }
+        unsafe {
+            let fd = Exception::try1(|p_fd| SWSSConsumerStateTable_getFd(self.ptr, p_fd))?;
+            let fd = BorrowedFd::borrow_raw(fd.try_into().unwrap());
+            Ok(fd)
+        }
     }
 
-    pub fn read_data(&self, timeout: Duration, interrupt_on_signal: bool) -> SelectResult {
+    pub fn read_data(&self, timeout: Duration, interrupt_on_signal: bool) -> Result<SelectResult> {
         let timeout_ms = timeout.as_millis().try_into().unwrap();
-        let res = unsafe { SWSSConsumerStateTable_readData(self.ptr, timeout_ms, interrupt_on_signal as u8) };
-        SelectResult::from_raw(res)
+        let res = unsafe {
+            Exception::try1(|p_res| {
+                SWSSConsumerStateTable_readData(self.ptr, timeout_ms, interrupt_on_signal as u8, p_res)
+            })?
+        };
+        Ok(SelectResult::from_raw(res))
     }
 }
 
 impl Drop for ConsumerStateTable {
     fn drop(&mut self) {
-        unsafe { SWSSConsumerStateTable_free(self.ptr) };
+        unsafe { Exception::try0(SWSSConsumerStateTable_free(self.ptr)).expect("Dropping ConsumerStateTable") };
     }
 }
 

crates/swss-common/src/types/cxxstring.rs

@@ -1,4 +1,4 @@
-use crate::*;
+use crate::bindings::*;
 use std::{
     borrow::{Borrow, Cow},
     fmt::Debug,