From 0190c25e9ddf3e1fe868870f6f94e541a19310f3 Mon Sep 17 00:00:00 2001 From: Christian Eltzschig Date: Thu, 19 Dec 2024 00:24:23 +0100 Subject: [PATCH] [#264] Add documentation and fix clippy warnings --- .../cxx/include/iox2/enum_translation.hpp | 113 ++++++++++-------- .../cxx/include/iox2/node_failure_enums.hpp | 2 + iceoryx2-ffi/cxx/include/iox2/node_id.hpp | 14 +-- iceoryx2-ffi/cxx/include/iox2/node_state.hpp | 10 +- iceoryx2-ffi/cxx/src/node_id.cpp | 2 +- iceoryx2-ffi/cxx/tests/src/node_tests.cpp | 2 +- iceoryx2-ffi/ffi/src/api/node.rs | 27 ++++- iceoryx2-ffi/ffi/src/api/node_id.rs | 51 +++++++- 8 files changed, 151 insertions(+), 70 deletions(-) diff --git a/iceoryx2-ffi/cxx/include/iox2/enum_translation.hpp b/iceoryx2-ffi/cxx/include/iox2/enum_translation.hpp index 82dd000db..c14db3c45 100644 --- a/iceoryx2-ffi/cxx/include/iox2/enum_translation.hpp +++ b/iceoryx2-ffi/cxx/include/iox2/enum_translation.hpp @@ -53,8 +53,9 @@ constexpr auto from(const int value) noexcept -> } template <> -constexpr auto from( - const iox2::SemanticStringError value) noexcept -> iox2_semantic_string_error_e { +constexpr auto +from(const iox2::SemanticStringError value) noexcept + -> iox2_semantic_string_error_e { switch (value) { case iox2::SemanticStringError::InvalidContent: return iox2_semantic_string_error_e_INVALID_CONTENT; @@ -85,8 +86,8 @@ constexpr auto from(const int value) noexcept -> iox2::S } template <> -constexpr auto -from(const iox2::ServiceType value) noexcept -> iox2_service_type_e { +constexpr auto from(const iox2::ServiceType value) noexcept + -> iox2_service_type_e { switch (value) { case iox2::ServiceType::Ipc: return iox2_service_type_e_IPC; @@ -111,8 +112,9 @@ constexpr auto from(const int value) noexcept -> } template <> -constexpr auto from( - const iox2::NodeCreationFailure value) noexcept -> iox2_node_creation_failure_e { +constexpr auto +from(const iox2::NodeCreationFailure value) noexcept + -> iox2_node_creation_failure_e { switch (value) { case iox2::NodeCreationFailure::InsufficientPermissions: return iox2_node_creation_failure_e_INSUFFICIENT_PERMISSIONS; @@ -143,8 +145,9 @@ constexpr auto from(const int value) noexcept -> } template <> -constexpr auto from( - const iox2::CallbackProgression value) noexcept -> iox2_callback_progression_e { +constexpr auto +from(const iox2::CallbackProgression value) noexcept + -> iox2_callback_progression_e { switch (value) { case iox2::CallbackProgression::Continue: return iox2_callback_progression_e_CONTINUE; @@ -258,8 +261,9 @@ constexpr auto from(const int value) noexcept -> } template <> -constexpr auto from( - const iox2::ServiceDetailsError value) noexcept -> iox2_service_details_error_e { +constexpr auto +from(const iox2::ServiceDetailsError value) noexcept + -> iox2_service_details_error_e { switch (value) { case iox2::ServiceDetailsError::FailedToOpenStaticServiceInfo: return iox2_service_details_error_e_FAILED_TO_OPEN_STATIC_SERVICE_INFO; @@ -337,8 +341,9 @@ constexpr auto from(const int value) noexcept } template <> -constexpr auto from( - const iox2::EventOpenOrCreateError value) noexcept -> iox2_event_open_or_create_error_e { +constexpr auto +from(const iox2::EventOpenOrCreateError value) noexcept + -> iox2_event_open_or_create_error_e { switch (value) { case iox2::EventOpenOrCreateError::OpenDoesNotExist: return iox2_event_open_or_create_error_e_O_DOES_NOT_EXIST; @@ -489,8 +494,9 @@ constexpr auto from(const int value) noexcept -> io } template <> -constexpr auto from( - const iox2::EventCreateError value) noexcept -> iox2_event_open_or_create_error_e { +constexpr auto +from(const iox2::EventCreateError value) noexcept + -> iox2_event_open_or_create_error_e { switch (value) { case iox2::EventCreateError::InsufficientPermissions: return iox2_event_open_or_create_error_e_C_INSUFFICIENT_PERMISSIONS; @@ -664,15 +670,14 @@ constexpr auto from -inline auto -from(const iox2::PublishSubscribeOpenError value) noexcept -> const - char* { +inline auto from(const iox2::PublishSubscribeOpenError value) noexcept + -> const char* { return iox2_pub_sub_open_or_create_error_string(iox::into(value)); } template <> -constexpr auto -from(const int value) noexcept -> iox2::PublishSubscribeCreateError { +constexpr auto from(const int value) noexcept + -> iox2::PublishSubscribeCreateError { const auto error = static_cast(value); switch (error) { case iox2_pub_sub_open_or_create_error_e_C_SERVICE_IN_CORRUPTED_STATE: @@ -718,9 +723,8 @@ constexpr auto from -inline auto -from(const iox2::PublishSubscribeCreateError value) noexcept -> const - char* { +inline auto from(const iox2::PublishSubscribeCreateError value) noexcept + -> const char* { return iox2_pub_sub_open_or_create_error_string(iox::into(value)); } @@ -784,8 +788,9 @@ constexpr auto from -inline auto from( - const iox2::PublishSubscribeOpenOrCreateError value) noexcept -> const char* { +inline auto +from(const iox2::PublishSubscribeOpenOrCreateError value) noexcept + -> const char* { return iox2_pub_sub_open_or_create_error_string(iox::into(value)); } @@ -801,8 +806,9 @@ constexpr auto from(const int value) noexcept -> } template <> -constexpr auto from( - const iox2::NotifierCreateError value) noexcept -> iox2_notifier_create_error_e { +constexpr auto +from(const iox2::NotifierCreateError value) noexcept + -> iox2_notifier_create_error_e { switch (value) { case iox2::NotifierCreateError::ExceedsMaxSupportedNotifiers: return iox2_notifier_create_error_e_EXCEEDS_MAX_SUPPORTED_NOTIFIERS; @@ -831,8 +837,9 @@ constexpr auto from(const int value) noexcept -> } template <> -constexpr auto from( - const iox2::ListenerCreateError value) noexcept -> iox2_listener_create_error_e { +constexpr auto +from(const iox2::ListenerCreateError value) noexcept + -> iox2_listener_create_error_e { switch (value) { case iox2::ListenerCreateError::ExceedsMaxSupportedListeners: return iox2_listener_create_error_e_EXCEEDS_MAX_SUPPORTED_LISTENERS; @@ -861,8 +868,9 @@ constexpr auto from(const int value) noexcept -> } template <> -constexpr auto from( - const iox2::NotifierNotifyError value) noexcept -> iox2_notifier_notify_error_e { +constexpr auto +from(const iox2::NotifierNotifyError value) noexcept + -> iox2_notifier_notify_error_e { switch (value) { case iox2::NotifierNotifyError::EventIdOutOfBounds: return iox2_notifier_notify_error_e_EVENT_ID_OUT_OF_BOUNDS; @@ -926,8 +934,9 @@ constexpr auto from(const int value) noexcept - } template <> -constexpr auto from( - const iox2::PublisherCreateError value) noexcept -> iox2_publisher_create_error_e { +constexpr auto +from(const iox2::PublisherCreateError value) noexcept + -> iox2_publisher_create_error_e { switch (value) { case iox2::PublisherCreateError::ExceedsMaxSupportedPublishers: return iox2_publisher_create_error_e_EXCEEDS_MAX_SUPPORTED_PUBLISHERS; @@ -958,8 +967,9 @@ constexpr auto from(const int value) noexcept } template <> -constexpr auto from( - const iox2::SubscriberCreateError value) noexcept -> iox2_subscriber_create_error_e { +constexpr auto +from(const iox2::SubscriberCreateError value) noexcept + -> iox2_subscriber_create_error_e { switch (value) { case iox2::SubscriberCreateError::BufferSizeExceedsMaxSupportedBufferSizeOfService: return iox2_subscriber_create_error_e_BUFFER_SIZE_EXCEEDS_MAX_SUPPORTED_BUFFER_SIZE_OF_SERVICE; @@ -1000,8 +1010,9 @@ constexpr auto from(const int value) noexcept -> } template <> -constexpr auto from( - const iox2::PublisherSendError value) noexcept -> iox2_publisher_send_error_e { +constexpr auto +from(const iox2::PublisherSendError value) noexcept + -> iox2_publisher_send_error_e { switch (value) { case iox2::PublisherSendError::ConnectionBrokenSincePublisherNoLongerExists: return iox2_publisher_send_error_e_CONNECTION_BROKEN_SINCE_PUBLISHER_NO_LONGER_EXISTS; @@ -1043,8 +1054,9 @@ constexpr auto from(const int value) noexcept } template <> -constexpr auto from( - const iox2::SubscriberReceiveError value) noexcept -> iox2_subscriber_receive_error_e { +constexpr auto +from(const iox2::SubscriberReceiveError value) noexcept + -> iox2_subscriber_receive_error_e { switch (value) { case iox2::SubscriberReceiveError::FailedToEstablishConnection: return iox2_subscriber_receive_error_e_FAILED_TO_ESTABLISH_CONNECTION; @@ -1081,8 +1093,9 @@ constexpr auto from(const int value) noexcept -> } template <> -constexpr auto from( - const iox2::PublisherLoanError value) noexcept -> iox2_publisher_loan_error_e { +constexpr auto +from(const iox2::PublisherLoanError value) noexcept + -> iox2_publisher_loan_error_e { switch (value) { case iox2::PublisherLoanError::ExceedsMaxLoanedSamples: return iox2_publisher_loan_error_e_EXCEEDS_MAX_LOANED_SAMPLES; @@ -1234,8 +1247,9 @@ constexpr auto from(const int value) noexcept -> } template <> -constexpr auto from( - const iox2::ConfigCreationError value) noexcept -> iox2_config_creation_error_e { +constexpr auto +from(const iox2::ConfigCreationError value) noexcept + -> iox2_config_creation_error_e { switch (value) { case iox2::ConfigCreationError::FailedToOpenConfigFile: return iox2_config_creation_error_e_FAILED_TO_OPEN_CONFIG_FILE; @@ -1306,8 +1320,9 @@ constexpr auto from(const int value) noexcept -> } template <> -constexpr auto from( - const iox2::WaitSetCreateError value) noexcept -> iox2_waitset_create_error_e { +constexpr auto +from(const iox2::WaitSetCreateError value) noexcept + -> iox2_waitset_create_error_e { switch (value) { case iox2::WaitSetCreateError::InternalError: return iox2_waitset_create_error_e_INTERNAL_ERROR; @@ -1371,8 +1386,9 @@ constexpr auto from(const int value) noexcept } template <> -constexpr auto from( - const iox2::WaitSetAttachmentError value) noexcept -> iox2_waitset_attachment_error_e { +constexpr auto +from(const iox2::WaitSetAttachmentError value) noexcept + -> iox2_waitset_attachment_error_e { switch (value) { case iox2::WaitSetAttachmentError::AlreadyAttached: return iox2_waitset_attachment_error_e_ALREADY_ATTACHED; @@ -1435,8 +1451,9 @@ inline auto from(const iox2::WaitSetRunError } template <> -constexpr auto from( - const iox2::SignalHandlingMode value) noexcept -> iox2_signal_handling_mode_e { +constexpr auto +from(const iox2::SignalHandlingMode value) noexcept + -> iox2_signal_handling_mode_e { switch (value) { case iox2::SignalHandlingMode::Disabled: return iox2_signal_handling_mode_e_DISABLED; diff --git a/iceoryx2-ffi/cxx/include/iox2/node_failure_enums.hpp b/iceoryx2-ffi/cxx/include/iox2/node_failure_enums.hpp index a27a994f4..f8d6ecf87 100644 --- a/iceoryx2-ffi/cxx/include/iox2/node_failure_enums.hpp +++ b/iceoryx2-ffi/cxx/include/iox2/node_failure_enums.hpp @@ -36,6 +36,8 @@ enum class NodeCreationFailure : uint8_t { InternalError }; +/// Failures of [`DeadNodeView::remove_stale_resources()`] that occur when the stale resources of +/// a dead [`Node`] are removed. enum class NodeCleanupFailure : uint8_t { /// The process received an interrupt signal while cleaning up all stale resources of a dead [`Node`]. Interrupt, diff --git a/iceoryx2-ffi/cxx/include/iox2/node_id.hpp b/iceoryx2-ffi/cxx/include/iox2/node_id.hpp index 2d78344c6..26f5e7271 100644 --- a/iceoryx2-ffi/cxx/include/iox2/node_id.hpp +++ b/iceoryx2-ffi/cxx/include/iox2/node_id.hpp @@ -21,6 +21,7 @@ #include namespace iox2 { +/// The system-wide unique id of a [`Node`] class NodeId { public: NodeId(const NodeId& rhs); @@ -29,10 +30,10 @@ class NodeId { auto operator=(NodeId&& rhs) noexcept -> NodeId&; ~NodeId(); - /// Returns high bits of the underlying value of the [`NodeId`]. + /// Returns the high bits of the underlying value of the [`NodeId`]. auto value_high() const -> uint64_t; - /// Returns low bits of the underlying value of the [`NodeId`]. + /// Returns the low bits of the underlying value of the [`NodeId`]. auto value_low() const -> uint64_t; /// Returns the [`ProcessId`] of the process that owns the [`Node`]. @@ -47,12 +48,9 @@ class NodeId { template friend class DeadNodeView; template - friend auto list_callback(iox2_node_state_e, - iox2_node_id_ptr, - const char*, - iox2_node_name_ptr, - iox2_config_ptr, - iox2_callback_context) -> iox2_callback_progression_e; + friend auto list_callback( + iox2_node_state_e, iox2_node_id_ptr, const char*, iox2_node_name_ptr, iox2_config_ptr, iox2_callback_context) + -> iox2_callback_progression_e; explicit NodeId(iox2_node_id_h handle); diff --git a/iceoryx2-ffi/cxx/include/iox2/node_state.hpp b/iceoryx2-ffi/cxx/include/iox2/node_state.hpp index 90124a08b..672876130 100644 --- a/iceoryx2-ffi/cxx/include/iox2/node_state.hpp +++ b/iceoryx2-ffi/cxx/include/iox2/node_state.hpp @@ -102,13 +102,9 @@ class NodeState { private: template - // NOLINTBEGIN(readability-function-size) - friend auto list_callback(iox2_node_state_e, - iox2_node_id_ptr, - const char*, - iox2_node_name_ptr, - iox2_config_ptr, - iox2_callback_context) -> iox2_callback_progression_e; + friend auto list_callback( + iox2_node_state_e, iox2_node_id_ptr, const char*, iox2_node_name_ptr, iox2_config_ptr, iox2_callback_context) + -> iox2_callback_progression_e; explicit NodeState(const AliveNodeView& view); explicit NodeState(const DeadNodeView& view); diff --git a/iceoryx2-ffi/cxx/src/node_id.cpp b/iceoryx2-ffi/cxx/src/node_id.cpp index 2f2da6a01..82e19f8e0 100644 --- a/iceoryx2-ffi/cxx/src/node_id.cpp +++ b/iceoryx2-ffi/cxx/src/node_id.cpp @@ -80,7 +80,7 @@ auto NodeId::creation_time() const -> timespec { uint32_t nanoseconds = 0; iox2_node_id_creation_time(&m_handle, &seconds, &nanoseconds); - return { static_cast(seconds), nanoseconds }; + return { static_cast(seconds), static_cast(nanoseconds) }; } auto operator<<(std::ostream& stream, const NodeId& node) -> std::ostream& { diff --git a/iceoryx2-ffi/cxx/tests/src/node_tests.cpp b/iceoryx2-ffi/cxx/tests/src/node_tests.cpp index 838758aab..85d6c9c60 100644 --- a/iceoryx2-ffi/cxx/tests/src/node_tests.cpp +++ b/iceoryx2-ffi/cxx/tests/src/node_tests.cpp @@ -70,7 +70,7 @@ TYPED_TEST(NodeTest, created_nodes_can_be_listed) { } uint64_t counter = 0; - auto result = Node::list(Config::global_config(), [&](auto node_state) { + auto result = Node::list(Config::global_config(), [&](const auto& node_state) { counter++; return CallbackProgression::Continue; }); diff --git a/iceoryx2-ffi/ffi/src/api/node.rs b/iceoryx2-ffi/ffi/src/api/node.rs index 0740261f0..48264c391 100644 --- a/iceoryx2-ffi/ffi/src/api/node.rs +++ b/iceoryx2-ffi/ffi/src/api/node.rs @@ -37,11 +37,15 @@ use super::{iox2_config_h_ref, iox2_node_id_h_ref, iox2_node_id_ptr, iox2_signal // BEGIN type definition +/// The failures that can occur when a list of node states is created with [`iox2_node_list()`]. #[repr(C)] #[derive(Copy, Clone, StringLiteral)] pub enum iox2_node_list_failure_e { + /// A list of all Nodes could not be created since the process does not have sufficient permissions. INSUFFICIENT_PERMISSIONS = IOX2_OK as isize + 1, + /// The process received an interrupt signal while acquiring the list of all Nodes. INTERRUPT, + /// Errors that indicate either an implementation issue or a wrongly configured system. INTERNAL_ERROR, } @@ -73,11 +77,16 @@ impl IntoCInt for NodeWaitFailure { } } +/// Failures of [`iox2_dead_node_remove_stale_resources()`] that occur when the stale resources of +/// a dead node are removed. #[repr(C)] #[derive(Copy, Clone, StringLiteral)] pub enum iox2_node_cleanup_failure_e { + /// The process received an interrupt signal while cleaning up all stale resources of a dead node. INTERRUPT = IOX2_OK as isize + 1, + /// Errors that indicate either an implementation issue or a wrongly configured system. INTERNAL_ERROR, + /// The stale resources of a dead node could not be removed since the process does not have sufficient permissions. INSUFFICIENT_PERMISSIONS, } @@ -348,6 +357,16 @@ pub unsafe extern "C" fn iox2_node_id( } } +/// Removes all stale resources of a dead node under a provided config. +/// +/// Returns [`IOX2_OK`] on success, otherwise [`iox2_node_cleanup_failure_e`]. +/// +/// # Safety +/// +/// * The `node_handle` must be valid and obtained by [`iox2_node_builder_create`](crate::iox2_node_builder_create)! +/// * The `node_id` must be valid +/// * The `config` must be valid +/// * `has_success` must point to a valid memory location #[no_mangle] pub unsafe extern "C" fn iox2_dead_node_remove_stale_resources( service_type: iox2_service_type_e, @@ -355,8 +374,12 @@ pub unsafe extern "C" fn iox2_dead_node_remove_stale_resources( config: iox2_config_h_ref, has_success: *mut bool, ) -> c_int { - let node_id = (&*node_id.as_type()).value.as_ref(); - let config = (&*config.as_type()).value.as_ref(); + node_id.assert_non_null(); + config.assert_non_null(); + debug_assert!(!has_success.is_null()); + + let node_id = (*node_id.as_type()).value.as_ref(); + let config = (*config.as_type()).value.as_ref(); let result = match service_type { iox2_service_type_e::IPC => { diff --git a/iceoryx2-ffi/ffi/src/api/node_id.rs b/iceoryx2-ffi/ffi/src/api/node_id.rs index db614e738..6d0fb2356 100644 --- a/iceoryx2-ffi/ffi/src/api/node_id.rs +++ b/iceoryx2-ffi/ffi/src/api/node_id.rs @@ -80,6 +80,15 @@ impl HandleToType for iox2_node_id_h_ref { // END type definition // BEGIN C API + +/// Creates a new [`iox2_node_id_h`] by cloning a [`iox2_node_id_ptr`]. +/// +/// # Safety +/// +/// * `node_id_struct_ptr` - Must be either a NULL pointer or a pointer to a valid [`iox2_node_id_t`]. +/// If it is a NULL pointer, the storage will be allocated on the heap. +/// * `node_id_ptr` - Must be a valid [`iox2_node_id_ptr`] +/// * `node_id_handle_ptr` - Must point to a valid [`iox2_node_id_h`]. #[no_mangle] pub unsafe extern "C" fn iox2_node_id_clone_from_ptr( node_id_struct_ptr: *mut iox2_node_id_t, @@ -104,10 +113,18 @@ pub unsafe extern "C" fn iox2_node_id_clone_from_ptr( (*node_id_struct_ptr).deleter = deleter; } - (*node_id_struct_ptr).value.init((*node_id_ptr).clone()); + (*node_id_struct_ptr).value.init(*node_id_ptr); *node_id_handle_ptr = (*node_id_struct_ptr).as_handle(); } +/// Creates a new [`iox2_node_id_h`] by cloning a [`iox2_node_id_h_ref`]. +/// +/// # Safety +/// +/// * `node_id_struct_ptr` - Must be either a NULL pointer or a pointer to a valid [`iox2_node_id_t`]. +/// If it is a NULL pointer, the storage will be allocated on the heap. +/// * `node_id_handle` - Must be a valid [`iox2_node_id_h_ref`] +/// * `node_id_handle_ptr` - Must point to a valid [`iox2_node_id_h`]. #[no_mangle] pub unsafe extern "C" fn iox2_node_id_clone_from_handle( node_id_struct_ptr: *mut iox2_node_id_t, @@ -123,6 +140,11 @@ pub unsafe extern "C" fn iox2_node_id_clone_from_handle( iox2_node_id_clone_from_ptr(node_id_struct_ptr, node_id_ptr, node_id_handle_ptr); } +/// Returns the high bits of the underlying value of the [`iox2_node_id_h`]. +/// +/// # Safety +/// +/// * `node_id_handle` - Must be a valid [`iox2_node_id_h_ref`] #[no_mangle] pub unsafe extern "C" fn iox2_node_id_value_high(node_id_handle: iox2_node_id_h_ref) -> u64 { node_id_handle.assert_non_null(); @@ -131,22 +153,39 @@ pub unsafe extern "C" fn iox2_node_id_value_high(node_id_handle: iox2_node_id_h_ (node_id.value.as_ref().value() >> 64) as u64 } +/// Returns the low bits of the underlying value of the [`iox2_node_id_h`]. +/// +/// # Safety +/// +/// * `node_id_handle` - Must be a valid [`iox2_node_id_h_ref`] #[no_mangle] pub unsafe extern "C" fn iox2_node_id_value_low(node_id_handle: iox2_node_id_h_ref) -> u64 { node_id_handle.assert_non_null(); let node_id = &mut *node_id_handle.as_type(); - ((node_id.value.as_ref().value() << 64) >> 64) as u64 + node_id.value.as_ref().value() as u64 } +/// Returns the process id of the [`iox2_node_id_h`]. +/// +/// # Safety +/// +/// * `node_id_handle` - Must be a valid [`iox2_node_id_h_ref`] #[no_mangle] pub unsafe extern "C" fn iox2_node_id_pid(node_id_handle: iox2_node_id_h_ref) -> i32 { node_id_handle.assert_non_null(); let node_id = &mut *node_id_handle.as_type(); - node_id.value.as_ref().pid().value() + node_id.value.as_ref().pid().value() as _ } +/// Returns the creation time of the [`iox2_node_id_h`]. +/// +/// # Safety +/// +/// * `node_id_handle` - Must be a valid [`iox2_node_id_h_ref`] +/// * `seconds` - Must point to a valid memory location +/// * `nanoseconds` - Must point to a valid memory location #[no_mangle] pub unsafe extern "C" fn iox2_node_id_creation_time( node_id_handle: iox2_node_id_h_ref, @@ -162,6 +201,12 @@ pub unsafe extern "C" fn iox2_node_id_creation_time( *nanoseconds = node_id.value.as_ref().creation_time().nanoseconds(); } +/// Takes ownership of the handle to delete and remove the underlying resources of a +/// [`iox2_node_id_h`]. +/// +/// # Safety +/// +/// * `node_id_handle` - Must be a valid [`iox2_node_id_h`] #[no_mangle] pub unsafe extern "C" fn iox2_node_id_drop(node_id_handle: iox2_node_id_h) { debug_assert!(!node_id_handle.is_null());