diff --git a/common/c-api/consumerstatetable.cpp b/common/c-api/consumerstatetable.cpp index 426055da1..524aa9854 100644 --- a/common/c-api/consumerstatetable.cpp +++ b/common/c-api/consumerstatetable.cpp @@ -38,8 +38,8 @@ SWSSResult SWSSConsumerStateTable_pops(SWSSConsumerStateTable tbl, }); } -SWSSResult SWSSConsumerStateTable_getFd(SWSSConsumerStateTable tbl, uint32_t *outFd) { - SWSSTry(*outFd = numeric_cast(((ConsumerStateTable *)tbl)->getFd())); +SWSSResult SWSSConsumerStateTable_getFd(SWSSConsumerStateTable tbl, int32_t *outFd) { + SWSSTry(*outFd = ((ConsumerStateTable *)tbl)->getFd()); } SWSSResult SWSSConsumerStateTable_readData(SWSSConsumerStateTable tbl, uint32_t timeout_ms, diff --git a/common/c-api/consumerstatetable.h b/common/c-api/consumerstatetable.h index 43ed4cde4..44ef08156 100644 --- a/common/c-api/consumerstatetable.h +++ b/common/c-api/consumerstatetable.h @@ -26,7 +26,7 @@ SWSSResult SWSSConsumerStateTable_pops(SWSSConsumerStateTable tbl, SWSSKeyOpFiel // Callers must NOT read/write on the fd, it may only be used for epoll or similar. // After the fd becomes readable, SWSSConsumerStateTable_readData must be used to // reset the fd and read data into internal data structures. -SWSSResult SWSSConsumerStateTable_getFd(SWSSConsumerStateTable tbl, uint32_t *outFd); +SWSSResult SWSSConsumerStateTable_getFd(SWSSConsumerStateTable tbl, int32_t *outFd); // Block until data is available to read or until a timeout elapses. // A timeout of 0 means the call will return immediately. diff --git a/common/c-api/subscriberstatetable.cpp b/common/c-api/subscriberstatetable.cpp index 3e1deb8ec..fdb563ac2 100644 --- a/common/c-api/subscriberstatetable.cpp +++ b/common/c-api/subscriberstatetable.cpp @@ -39,8 +39,8 @@ SWSSResult SWSSSubscriberStateTable_pops(SWSSSubscriberStateTable tbl, }); } -SWSSResult SWSSSubscriberStateTable_getFd(SWSSSubscriberStateTable tbl, uint32_t *outFd) { - SWSSTry(*outFd = numeric_cast(((SubscriberStateTable *)tbl)->getFd())); +SWSSResult SWSSSubscriberStateTable_getFd(SWSSSubscriberStateTable tbl, int32_t *outFd) { + SWSSTry(*outFd = ((SubscriberStateTable *)tbl)->getFd()); } SWSSResult SWSSSubscriberStateTable_readData(SWSSSubscriberStateTable tbl, uint32_t timeout_ms, diff --git a/common/c-api/subscriberstatetable.h b/common/c-api/subscriberstatetable.h index 5f7444063..41b37235b 100644 --- a/common/c-api/subscriberstatetable.h +++ b/common/c-api/subscriberstatetable.h @@ -28,7 +28,7 @@ SWSSResult SWSSSubscriberStateTable_pops(SWSSSubscriberStateTable tbl, // Callers must NOT read/write on the fd, it may only be used for epoll or similar. // After the fd becomes readable, SWSSSubscriberStateTable_readData must be used to // reset the fd and read data into internal data structures. -SWSSResult SWSSSubscriberStateTable_getFd(SWSSSubscriberStateTable tbl, uint32_t *outFd); +SWSSResult SWSSSubscriberStateTable_getFd(SWSSSubscriberStateTable tbl, int32_t *outFd); // Block until data is available to read or until a timeout elapses. // A timeout of 0 means the call will return immediately. diff --git a/common/c-api/zmqconsumerstatetable.cpp b/common/c-api/zmqconsumerstatetable.cpp index 59ed7dde4..48a000950 100644 --- a/common/c-api/zmqconsumerstatetable.cpp +++ b/common/c-api/zmqconsumerstatetable.cpp @@ -35,8 +35,8 @@ SWSSResult SWSSZmqConsumerStateTable_pops(SWSSZmqConsumerStateTable tbl, }); } -SWSSResult SWSSZmqConsumerStateTable_getFd(SWSSZmqConsumerStateTable tbl, uint32_t *outFd) { - SWSSTry(*outFd = numeric_cast(((ZmqConsumerStateTable *)tbl)->getFd())); +SWSSResult SWSSZmqConsumerStateTable_getFd(SWSSZmqConsumerStateTable tbl, int32_t *outFd) { + SWSSTry(*outFd = ((ZmqConsumerStateTable *)tbl)->getFd()); } SWSSResult SWSSZmqConsumerStateTable_readData(SWSSZmqConsumerStateTable tbl, uint32_t timeout_ms, diff --git a/common/c-api/zmqconsumerstatetable.h b/common/c-api/zmqconsumerstatetable.h index f781f6bc8..7a25053f2 100644 --- a/common/c-api/zmqconsumerstatetable.h +++ b/common/c-api/zmqconsumerstatetable.h @@ -29,7 +29,7 @@ SWSSResult SWSSZmqConsumerStateTable_pops(SWSSZmqConsumerStateTable tbl, // Callers must NOT read/write on fd, it may only be used for epoll or similar. // After the fd becomes readable, SWSSZmqConsumerStateTable_readData must be used to // reset the fd and read data into internal data structures. -SWSSResult SWSSZmqConsumerStateTable_getFd(SWSSZmqConsumerStateTable tbl, uint32_t *outFd); +SWSSResult SWSSZmqConsumerStateTable_getFd(SWSSZmqConsumerStateTable tbl, int32_t *outFd); // Block until data is available to read or until a timeout elapses. // A timeout of 0 means the call will return immediately. diff --git a/crates/swss-common/src/types/consumerstatetable.rs b/crates/swss-common/src/types/consumerstatetable.rs index 7789b72cc..aa188d8fb 100644 --- a/crates/swss-common/src/types/consumerstatetable.rs +++ b/crates/swss-common/src/types/consumerstatetable.rs @@ -38,13 +38,17 @@ impl ConsumerStateTable { // as long as the DbConnector does. unsafe { let fd = swss_try!(p_fd => SWSSConsumerStateTable_getFd(self.ptr, p_fd))?; - let fd = BorrowedFd::borrow_raw(fd.try_into().unwrap()); + if fd == -1 { + return Err(Exception::new("Invalid file descriptor: -1")); + } + let fd = BorrowedFd::borrow_raw(fd); Ok(fd) } } pub fn read_data(&self, timeout: Duration, interrupt_on_signal: bool) -> Result { - let timeout_ms = timeout.as_millis().try_into().unwrap(); + let timeout_ms: u32 = timeout.as_millis().try_into() + .map_err(|_| Exception::new("Invalid timeout value"))?; let res = unsafe { swss_try!(p_res => { SWSSConsumerStateTable_readData(self.ptr, timeout_ms, interrupt_on_signal as u8, p_res) @@ -68,7 +72,11 @@ impl ConsumerStateTable { impl Drop for ConsumerStateTable { fn drop(&mut self) { - unsafe { swss_try!(SWSSConsumerStateTable_free(self.ptr)).expect("Dropping ConsumerStateTable") }; + unsafe { + if let Err(e) = swss_try!(SWSSConsumerStateTable_free(self.ptr)) { + eprintln!("Error dropping ConsumerStateTable: {}", e); + } + } } } diff --git a/crates/swss-common/src/types/dbconnector.rs b/crates/swss-common/src/types/dbconnector.rs index 152adedf7..8ce8caed1 100644 --- a/crates/swss-common/src/types/dbconnector.rs +++ b/crates/swss-common/src/types/dbconnector.rs @@ -208,20 +208,13 @@ impl DbConnector { } } -impl Clone for DbConnector { - /// Clone with a default timeout of 15 seconds. - /// - /// 15 seconds was picked as an absurdly long time to wait for Redis to respond. - /// Panics after a timeout, or if any other exception occurred. - /// Use `clone_timeout` for control of timeout and exception handling. - fn clone(&self) -> Self { - self.clone_timeout(15000).expect("DbConnector::clone failed") - } -} - impl Drop for DbConnector { fn drop(&mut self) { - unsafe { swss_try!(SWSSDBConnector_free(self.ptr)).expect("Dropping DbConnector") }; + unsafe { + if let Err(e) = swss_try!(SWSSDBConnector_free(self.ptr)) { + eprintln!("Error dropping DbConnector: {}", e); + } + } } } @@ -245,5 +238,4 @@ impl DbConnector { async_util::impl_basic_async_method!(hgetall_async <= hgetall(&self, key: &str) -> Result>); async_util::impl_basic_async_method!(hexists_async <= hexists(&self, key: &str, field: &str) -> Result); async_util::impl_basic_async_method!(flush_db_async <= flush_db(&self) -> Result); - async_util::impl_basic_async_method!(clone_async <= clone(&self) -> Self); } diff --git a/crates/swss-common/src/types/exception.rs b/crates/swss-common/src/types/exception.rs index bc7a61122..6e18373c7 100644 --- a/crates/swss-common/src/types/exception.rs +++ b/crates/swss-common/src/types/exception.rs @@ -59,6 +59,36 @@ impl Exception { } } + /// Create a new Exception with a custom message (for Rust-side errors). + /// Automatically captures the caller's file and line number using `#[track_caller]`. + /// + /// # Example + /// ``` + /// use swss_common::{Exception, Result}; + /// + /// fn validate_fd(fd: i32) -> Result<()> { + /// if fd < 0 { + /// return Err(Exception::new("Invalid file descriptor")); + /// } + /// Ok(()) + /// } + /// + /// // The exception will automatically include the location where it was created: + /// let result = validate_fd(-1); + /// assert!(result.is_err()); + /// let err = result.unwrap_err(); + /// assert_eq!(err.message(), "Invalid file descriptor"); + /// // err.location() will be something like "src/types/exception.rs:70" + /// ``` + #[track_caller] + pub fn new(message: impl Into) -> Self { + let location = std::panic::Location::caller(); + Self { + message: message.into(), + location: format!("{}:{}", location.file(), location.line()), + } + } + /// Get an informational string about the error that occurred. pub fn message(&self) -> &str { &self.message diff --git a/crates/swss-common/src/types/producerstatetable.rs b/crates/swss-common/src/types/producerstatetable.rs index 6bfbbe6cd..1ce97fbd3 100644 --- a/crates/swss-common/src/types/producerstatetable.rs +++ b/crates/swss-common/src/types/producerstatetable.rs @@ -58,7 +58,11 @@ impl ProducerStateTable { impl Drop for ProducerStateTable { fn drop(&mut self) { - unsafe { swss_try!(SWSSProducerStateTable_free(self.ptr)).expect("Dropping ProducerStateTable") }; + unsafe { + if let Err(e) = swss_try!(SWSSProducerStateTable_free(self.ptr)) { + eprintln!("Error dropping ProducerStateTable: {}", e); + } + } } } diff --git a/crates/swss-common/src/types/subscriberstatetable.rs b/crates/swss-common/src/types/subscriberstatetable.rs index c72eae3cc..a3e3989f9 100644 --- a/crates/swss-common/src/types/subscriberstatetable.rs +++ b/crates/swss-common/src/types/subscriberstatetable.rs @@ -36,7 +36,8 @@ impl SubscriberStateTable { } pub fn read_data(&self, timeout: Duration, interrupt_on_signal: bool) -> Result { - let timeout_ms = timeout.as_millis().try_into().unwrap(); + let timeout_ms: u32 = timeout.as_millis().try_into() + .map_err(|_| Exception::new("Invalid timeout value"))?; let res = unsafe { swss_try!(p_res => { SWSSSubscriberStateTable_readData(self.ptr, timeout_ms, interrupt_on_signal as u8, p_res) @@ -50,7 +51,10 @@ impl SubscriberStateTable { // as long as the DbConnector does. unsafe { let fd = swss_try!(p_fd => SWSSSubscriberStateTable_getFd(self.ptr, p_fd))?; - let fd = BorrowedFd::borrow_raw(fd.try_into().unwrap()); + if fd == -1 { + return Err(Exception::new("Invalid file descriptor: -1")); + } + let fd = BorrowedFd::borrow_raw(fd); Ok(fd) } } @@ -70,7 +74,11 @@ impl SubscriberStateTable { impl Drop for SubscriberStateTable { fn drop(&mut self) { - unsafe { swss_try!(SWSSSubscriberStateTable_free(self.ptr)).expect("Dropping SubscriberStateTable") }; + unsafe { + if let Err(e) = swss_try!(SWSSSubscriberStateTable_free(self.ptr)) { + eprintln!("Error dropping SubscriberStateTable: {}", e); + } + } } } diff --git a/crates/swss-common/src/types/table.rs b/crates/swss-common/src/types/table.rs index 6aec3af0f..abbde35b0 100644 --- a/crates/swss-common/src/types/table.rs +++ b/crates/swss-common/src/types/table.rs @@ -94,7 +94,11 @@ impl Table { impl Drop for Table { fn drop(&mut self) { - unsafe { swss_try!(SWSSTable_free(self.ptr)).expect("Dropping Table") }; + unsafe { + if let Err(e) = swss_try!(SWSSTable_free(self.ptr)) { + eprintln!("Error dropping Table: {}", e); + } + } } } diff --git a/crates/swss-common/src/types/zmqclient.rs b/crates/swss-common/src/types/zmqclient.rs index 489c10ea7..4046a5982 100644 --- a/crates/swss-common/src/types/zmqclient.rs +++ b/crates/swss-common/src/types/zmqclient.rs @@ -43,7 +43,11 @@ impl ZmqClient { impl Drop for ZmqClient { fn drop(&mut self) { - unsafe { swss_try!(SWSSZmqClient_free(self.ptr)).expect("Dropping ZmqClient") }; + unsafe { + if let Err(e) = swss_try!(SWSSZmqClient_free(self.ptr)) { + eprintln!("Error dropping ZmqClient: {}", e); + } + } } } diff --git a/crates/swss-common/src/types/zmqconsumerstatetable.rs b/crates/swss-common/src/types/zmqconsumerstatetable.rs index 0488b7677..e69e4c256 100644 --- a/crates/swss-common/src/types/zmqconsumerstatetable.rs +++ b/crates/swss-common/src/types/zmqconsumerstatetable.rs @@ -45,21 +45,25 @@ impl ZmqConsumerStateTable { } pub fn get_fd(&self) -> Result { - // SAFETY: This fd represents the underlying zmq socket, which should remain alive as long as there - // is a listener (i.e. a ZmqConsumerStateTable) + // SAFETY: This fd represents the underlying ZMQ socket, which should stay alive + // as long as this object does. unsafe { let fd = swss_try!(p_fd => SWSSZmqConsumerStateTable_getFd(self.ptr, p_fd))?; - let fd = BorrowedFd::borrow_raw(fd.try_into().unwrap()); + if fd == -1 { + return Err(Exception::new("Invalid file descriptor: -1")); + } + let fd = BorrowedFd::borrow_raw(fd); Ok(fd) } } pub fn read_data(&self, timeout: Duration, interrupt_on_signal: bool) -> Result { - let timeout_ms = timeout.as_millis().try_into().unwrap(); + let timeout_ms: u32 = timeout.as_millis().try_into() + .map_err(|_| Exception::new("Invalid timeout value"))?; let res = unsafe { - swss_try!(p_res => + swss_try!(p_res => { SWSSZmqConsumerStateTable_readData(self.ptr, timeout_ms, interrupt_on_signal as u8, p_res) - )? + })? }; Ok(SelectResult::from_raw(res)) } @@ -74,7 +78,11 @@ pub(crate) struct DropGuard(SWSSZmqConsumerStateTable); impl Drop for DropGuard { fn drop(&mut self) { - unsafe { swss_try!(SWSSZmqConsumerStateTable_free(self.0)).expect("Dropping ZmqConsumerStateTable") }; + unsafe { + if let Err(e) = swss_try!(SWSSZmqConsumerStateTable_free(self.0)) { + eprintln!("Error dropping ZmqConsumerStateTable: {}", e); + } + } } } diff --git a/tests/c_api_ut.cpp b/tests/c_api_ut.cpp index 1d5f87bc7..b4877b001 100644 --- a/tests/c_api_ut.cpp +++ b/tests/c_api_ut.cpp @@ -369,7 +369,7 @@ TEST(c_api, ConsumerProducerStateTables) { SWSSConsumerStateTable cst; SWSSConsumerStateTable_new(db, "mytable", nullptr, nullptr, &cst); - uint32_t fd; + int32_t fd; SWSSConsumerStateTable_getFd(cst, &fd); SWSSKeyOpFieldValuesArray arr; @@ -453,7 +453,7 @@ TEST(c_api, SubscriberStateTable) { SWSSSubscriberStateTable sst; SWSSSubscriberStateTable_new(db, "mytable", nullptr, nullptr, &sst); - uint32_t fd; + int32_t fd; SWSSSubscriberStateTable_getFd(sst, &fd); SWSSSelectResult result; @@ -506,7 +506,7 @@ TEST(c_api, ZmqConsumerProducerStateTable) { SWSSZmqConsumerStateTable cst; SWSSZmqConsumerStateTable_new(db, "mytable", srv, nullptr, nullptr, &cst); - uint32_t fd; + int32_t fd; SWSSZmqConsumerStateTable_getFd(cst, &fd); const SWSSDBConnectorOpaque *dbConnector;