[action] [PR:1113] Fix risky unwrap(), expect(), and casting

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<uint32_t>(((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,

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.

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<uint32_t>(((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,

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.

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<uint32_t>(((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,

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.

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<SelectResult> {
-        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);
+            }
+        }
     }
 }
 

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<HashMap<String, CxxString>>);
     async_util::impl_basic_async_method!(hexists_async <= hexists(&self, key: &str, field: &str) -> Result<bool>);
     async_util::impl_basic_async_method!(flush_db_async <= flush_db(&self) -> Result<bool>);
-    async_util::impl_basic_async_method!(clone_async <= clone(&self) -> Self);
 }

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<String>) -> 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

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);
+            }
+        }
     }
 }
 

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<SelectResult> {
-        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);
+            }
+        }
     }
 }
 

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);
+            }
+        }
     }
 }
 

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);
+            }
+        }
     }
 }
 

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

@@ -45,21 +45,25 @@ impl ZmqConsumerStateTable {
     }
 
     pub fn get_fd(&self) -> Result<BorrowedFd> {
-        // 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<SelectResult> {
-        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);
+            }
+        }
     }
 }
 

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;