Minor cleanups

Signed-off-by: Luca Della Vedova <[email protected]>

Author: luca-della-vedova

rclrs/src/dynamic_message.rs

@@ -70,7 +70,9 @@ pub struct DynamicMessageMetadata {
     introspection_type_support_library: Arc<libloading::Library>,
     type_support_ptr: *const rosidl_message_type_support_t,
     structure: MessageStructure,
-    fini_function: unsafe extern "C" fn(*mut std::os::raw::c_void),
+    // The message content can be moved out into another message,
+    // in which case the drop impl is not responsible for calling fini anymore
+    fini_function: Option<unsafe extern "C" fn(*mut std::os::raw::c_void)>,
 }
 
 /// A message whose type is not known at compile-time.
@@ -85,9 +87,6 @@ pub struct DynamicMessage {
     // This is aligned to the maximum possible alignment of a message (8)
     // by the use of a special allocation function.
     storage: Box<[u8]>,
-    // This type allows moving the message contents out into another message,
-    // in which case the drop impl is not responsible for calling fini anymore
-    needs_fini: bool,
 }
 
 // ========================= impl for DynamicMessagePackage =========================
@@ -201,16 +200,14 @@ impl DynamicMessagePackage {
             unsafe { &*(type_support.data as *const rosidl_message_members_t) };
         // SAFETY: The message members coming from a type support library will always be valid.
         let structure = unsafe { MessageStructure::from_rosidl_message_members(message_members) };
-        // The fini function will always exist.
-        let fini_function = message_members.fini_function.unwrap();
         let metadata = DynamicMessageMetadata {
             message_type,
             introspection_type_support_library: Arc::clone(
                 &self.introspection_type_support_library,
             ),
             type_support_ptr,
             structure,
-            fini_function,
+            fini_function: message_members.fini_function,
         };
         Ok(metadata)
     }
@@ -314,7 +311,6 @@ impl DynamicMessageMetadata {
         let dyn_msg = DynamicMessage {
             metadata: self.clone(),
             storage,
-            needs_fini: true,
         };
         Ok(dyn_msg)
     }
@@ -336,9 +332,9 @@ impl Deref for DynamicMessage {
 
 impl Drop for DynamicMessage {
     fn drop(&mut self) {
-        if self.needs_fini {
+        if let Some(fini_function) = self.metadata.fini_function {
             // SAFETY: The fini_function expects to be passed a pointer to the message
-            unsafe { (self.metadata.fini_function)(self.storage.as_mut_ptr() as _) }
+            unsafe { (fini_function)(self.storage.as_mut_ptr() as _) }
         }
     }
 }
@@ -475,7 +471,7 @@ impl DynamicMessage {
             dest_slice.copy_from_slice(&self.storage);
             // Don't run the fini function on the src data anymore, because the inner parts would be
             // double-freed by dst and src.
-            self.needs_fini = false;
+            self.metadata.fini_function = None;
             Ok(dest)
         } else {
             Err(self)

rclrs/src/dynamic_message/dynamic_subscription.rs

@@ -1,6 +1,6 @@
 use std::any::Any;
 use std::boxed::Box;
-use std::ffi::{CStr, CString};
+use std::ffi::CString;
 use std::ops::{Deref, DerefMut};
 use std::sync::{Arc, Mutex};
 
@@ -102,7 +102,7 @@ impl<Payload> DerefMut for WorkerDynamicSubscriptionCallback<Payload> {
     }
 }
 
-pub enum DynamicSubscriptionCallback<Payload> {
+pub(crate) enum DynamicSubscriptionCallback<Payload> {
     /// A callback with the message and the message info as arguments.
     Node(NodeAsyncDynamicSubscriptionCallback),
     /// A callback with the payload, message, and the message info as arguments.