Make rclrs panic-free as far as possible (#190)

This is achieved by making CString errors a part of RclrsError, since RclrsError is now an enum.

Author: nnmm

rclrs/src/context.rs

@@ -59,16 +59,18 @@ impl Context {
     /// let invalid_remapping = ["--ros-args", "-r", ":=:*/]"].map(String::from);
     /// assert!(Context::new(invalid_remapping).is_err());
     /// ```
-    ///
-    /// # Panics
-    /// When there is an interior null byte in any of the args.
     pub fn new(args: impl IntoIterator<Item = String>) -> Result<Self, RclrsError> {
         // SAFETY: Getting a zero-initialized value is always safe
         let mut rcl_context = unsafe { rcl_get_zero_initialized_context() };
         let cstring_args: Vec<CString> = args
             .into_iter()
-            .map(|arg| CString::new(arg).unwrap())
-            .collect();
+            .map(|arg| {
+                CString::new(arg.as_str()).map_err(|err| RclrsError::StringContainsNul {
+                    err,
+                    s: arg.clone(),
+                })
+            })
+            .collect::<Result<_, _>>()?;
         // Vector of pointers into cstring_args
         let c_args: Vec<*const c_char> = cstring_args.iter().map(|arg| arg.as_ptr()).collect();
         unsafe {

rclrs/src/error.rs

@@ -1,6 +1,6 @@
 use crate::rcl_bindings::*;
 use std::error::Error;
-use std::ffi::CStr;
+use std::ffi::{CStr, NulError};
 use std::fmt::{self, Display};
 
 /// The main error type.
@@ -20,13 +20,23 @@ pub enum RclrsError {
         /// The error message set in the `rcl` layer or below.
         msg: Option<RclErrorMsg>,
     },
+    /// A string provided to `rclrs` could not be converted into a `CString`.
+    StringContainsNul {
+        /// The string that contains a nul byte.
+        s: String,
+        /// The error indicating the position of the nul byte.
+        err: NulError,
+    },
 }
 
 impl Display for RclrsError {
     fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
         match self {
             RclrsError::RclError { code, .. } => write!(f, "{}", code),
             RclrsError::UnknownRclError { code, .. } => write!(f, "{}", code),
+            RclrsError::StringContainsNul { s, .. } => {
+                write!(f, "Could not convert string '{}' to CString", s)
+            }
         }
     }
 }
@@ -54,11 +64,11 @@ impl Error for RclErrorMsg {}
 
 impl Error for RclrsError {
     fn source(&self) -> Option<&(dyn Error + 'static)> {
-        let msg = match self {
-            RclrsError::RclError { msg, .. } => msg,
-            RclrsError::UnknownRclError { msg, .. } => msg,
-        };
-        msg.as_ref().map(|e| e as &dyn Error)
+        match self {
+            RclrsError::RclError { msg, .. } => msg.as_ref().map(|e| e as &dyn Error),
+            RclrsError::UnknownRclError { msg, .. } => msg.as_ref().map(|e| e as &dyn Error),
+            RclrsError::StringContainsNul { err, .. } => Some(err).map(|e| e as &dyn Error),
+        }
     }
 }
 

rclrs/src/node/builder.rs

@@ -148,13 +148,18 @@ impl NodeBuilder {
     ///
     /// For example usage, see the [`NodeBuilder`][1] docs.
     ///
-    /// # Panics
-    /// When the node name or namespace contain null bytes.
-    ///
     /// [1]: crate::NodeBuilder
     pub fn build(&self) -> Result<Node, RclrsError> {
-        let node_name = CString::new(self.name.as_str()).unwrap();
-        let node_namespace = CString::new(self.namespace.as_str()).unwrap();
+        let node_name =
+            CString::new(self.name.as_str()).map_err(|err| RclrsError::StringContainsNul {
+                err,
+                s: self.name.clone(),
+            })?;
+        let node_namespace =
+            CString::new(self.namespace.as_str()).map_err(|err| RclrsError::StringContainsNul {
+                err,
+                s: self.namespace.clone(),
+            })?;
 
         // SAFETY: No preconditions for this function.
         let mut node_handle = unsafe { rcl_get_zero_initialized_node() };

rclrs/src/node/publisher.rs

@@ -63,9 +63,6 @@ where
     /// Creates a new `Publisher`.
     ///
     /// Node and namespace changes are always applied _before_ topic remapping.
-    ///
-    /// # Panics
-    /// When the topic contains interior null bytes.
     pub fn new(node: &Node, topic: &str, qos: QoSProfile) -> Result<Self, RclrsError>
     where
         T: Message,
@@ -74,7 +71,10 @@ where
         let mut publisher_handle = unsafe { rcl_get_zero_initialized_publisher() };
         let type_support =
             <T as Message>::RmwMsg::get_type_support() as *const rosidl_message_type_support_t;
-        let topic_c_string = CString::new(topic).unwrap();
+        let topic_c_string = CString::new(topic).map_err(|err| RclrsError::StringContainsNul {
+            err,
+            s: topic.into(),
+        })?;
         let node_handle = &mut *node.handle.lock();
 
         // SAFETY: No preconditions for this function.

rclrs/src/node/subscription.rs

@@ -74,9 +74,6 @@ where
     T: Message,
 {
     /// Creates a new subscription.
-    ///
-    /// # Panics
-    /// When the topic contains interior null bytes.
     pub fn new<F>(
         node: &Node,
         topic: &str,
@@ -91,7 +88,10 @@ where
         let mut subscription_handle = unsafe { rcl_get_zero_initialized_subscription() };
         let type_support =
             <T as Message>::RmwMsg::get_type_support() as *const rosidl_message_type_support_t;
-        let topic_c_string = CString::new(topic).unwrap();
+        let topic_c_string = CString::new(topic).map_err(|err| RclrsError::StringContainsNul {
+            err,
+            s: topic.into(),
+        })?;
         let node_handle = &mut *node.handle.lock();
 
         // SAFETY: No preconditions for this function.