Reapply "Fix risky unwrap(), expect(), and casting (sonic-net#1113)" (sonic-net#1118)

This reverts commit 7682785.

Author: qiluo-msft

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