diff --git a/rmw/include/rmw/get_node_info_and_types.h b/rmw/include/rmw/get_node_info_and_types.h index 0b6da298..5987c888 100644 --- a/rmw/include/rmw/get_node_info_and_types.h +++ b/rmw/include/rmw/get_node_info_and_types.h @@ -25,9 +25,9 @@ extern "C" #include "rmw/types.h" #include "rmw/visibility_control.h" -/// Return all topic names and types for which a given remote node has subscriptions. +/// Return all topic names, types, and type hashes for which a given remote node has subscriptions. /** - * This function returns an array of topic names and types for which a given remote + * This function returns an array of topic names, types, and type hashes for which a given remote * node has subscriptions, as discovered so far by the given local node. * *
@@ -49,8 +49,8 @@ extern "C" * \par Thread-safety * Nodes are thread-safe objects, and so are all operations on them except for finalization. * Therefore, it is safe to query the ROS graph using the same node concurrently. - * However, when querying subscribed topic names and types: - * - Access to the array of names and types is not synchronized. + * However, when querying subscribed topic names, types, and type hashes: + * - Access to the array of names, types, and type hashes is not synchronized. * It is not safe to read or write `topic_names_and_types` * while rmw_get_subscriber_names_and_types_by_node() uses it. * - Access to node name and namespace is read-only but it is not synchronized. @@ -60,7 +60,7 @@ extern "C" * Check your allocator documentation for further reference. * * \pre Given `node` must be a valid node handle, as returned by rmw_create_node(). - * \pre Given `topic_names_and_types` must be a zero-initialized array of names and types, + * \pre Given `topic_names_and_types` must be a zero-initialized array of names, types, and type hashes, * as returned by rmw_get_zero_initialized_names_and_types(). * * \param[in] node Local node to query the ROS graph. @@ -68,7 +68,7 @@ extern "C" * \param[in] node_name Name of the remote node to get information for. * \param[in] node_namespace Namespace of the remote node to get information for. * \param[in] no_demangle Whether to demangle all topic names following ROS conventions or not. - * \param[out] topic_names_and_types Array of topic names and types the remote node has created + * \param[out] topic_names_and_types Array of topic names, types, and type hashes the remote node has created * a subscription for, populated on success but left unchanged on failure. * If populated, it is up to the caller to finalize this array later on * using rmw_names_and_types_fini(). @@ -100,9 +100,9 @@ rmw_get_subscriber_names_and_types_by_node( bool no_demangle, rmw_names_and_types_t * topic_names_and_types); -/// Return all topic names and types for which a given remote node has publishers. +/// Return all topic names, types, and type hashes for which a given remote node has publishers. /** - * This function returns an array of topic names and types for which a given remote + * This function returns an array of topic names, types, and type hashes for which a given remote * node has created publishers, as discovered so far by the given local node. * *
@@ -124,8 +124,8 @@ rmw_get_subscriber_names_and_types_by_node( * \par Thread-safety * Nodes are thread-safe objects, and so are all operations on them except for finalization. * Therefore, it is safe to query the ROS graph using the same node concurrently. - * However, when querying published topic names and types: - * - Access to the array of names and types is not synchronized. + * However, when querying published topic names, types, and type hashes: + * - Access to the array of names, types, and type hashes is not synchronized. * It is not safe to read or write `topic_names_and_types` * while rmw_get_publisher_names_and_types_by_node() uses it. * - Access to node name and namespace is read-only but it is not synchronized. @@ -135,7 +135,7 @@ rmw_get_subscriber_names_and_types_by_node( * Check your allocator documentation for further reference. * * \pre Given `node` must be a valid node handle, as returned by rmw_create_node(). - * \pre Given `topic_names_and_types` must be a zero-initialized array of names and types, + * \pre Given `topic_names_and_types` must be a zero-initialized array of names, types, and type hashes, * as returned by rmw_get_zero_initialized_names_and_types(). * * \param[in] node Local node to query the ROS graph. @@ -143,7 +143,7 @@ rmw_get_subscriber_names_and_types_by_node( * \param[in] node_name Name of the remote node to get information for. * \param[in] node_namespace Namespace of the remote node to get information for. * \param[in] no_demangle Whether to demangle all topic names following ROS conventions or not. - * \param[out] topic_names_and_types Array of topic names and types the remote node has created + * \param[out] topic_names_and_types Array of topic names, types, and type hashes the remote node has created * a publisher for, populated on success but left unchanged on failure. * If populated, it is up to the caller to finalize this array later on * using rmw_names_and_types_fini(). @@ -175,9 +175,9 @@ rmw_get_publisher_names_and_types_by_node( bool no_demangle, rmw_names_and_types_t * topic_names_and_types); -/// Return all service names and types for which a given remote node has servers. +/// Return all service names, types, and type hashes for which a given remote node has servers. /** - * This function returns an array of service names and types for which a given remote + * This function returns an array of service names, types, and type hashes for which a given remote * node has servers, as discovered so far by the given local node. * *
@@ -199,8 +199,8 @@ rmw_get_publisher_names_and_types_by_node( * \par Thread-safety * Nodes are thread-safe objects, and so are all operations on them except for finalization. * Therefore, it is safe to query the ROS graph using the same node concurrently. - * However, when querying served service names and types: - * - Access to the array of names and types is not synchronized. + * However, when querying served service names, types, and type hashes: + * - Access to the array of names, types, and type hashes is not synchronized. * It is not safe to read or write `service_names_and_types` * while rmw_get_service_names_and_types_by_node() uses it. * - Access to node name and namespace is read-only but it is not synchronized. @@ -213,7 +213,7 @@ rmw_get_publisher_names_and_types_by_node( * \param[in] allocator Allocator to be used when populating the `service_names_and_types` array. * \param[in] node_name Name of the remote node to get information for. * \param[in] node_namespace Namespace of the remote node to get information for. - * \param[out] service_names_and_types Array of service names and types the remote node has + * \param[out] service_names_and_types Array of service names, types, and type hashes the remote node has * created a service server for, populated on success but left unchanged on failure. * If populated, it is up to the caller to finalize this array later on * using rmw_names_and_types_fini(). @@ -244,9 +244,9 @@ rmw_get_service_names_and_types_by_node( const char * node_namespace, rmw_names_and_types_t * service_names_and_types); -/// Return all service names and types for which a given remote node has clients. +/// Return all service names, types, and type hashes for which a given remote node has clients. /** - * This function returns an array of service names and types for which a given remote + * This function returns an array of service names, types, and type hashes for which a given remote * node has clients, as discovered so far by the given local node. * *
@@ -268,8 +268,8 @@ rmw_get_service_names_and_types_by_node( * \par Thread-safety * Nodes are thread-safe objects, and so are all operations on them except for finalization. * Therefore, it is safe to query the ROS graph using the same node concurrently. - * However, when querying served service names and types: - * - Access to the array of names and types is not synchronized. + * However, when querying served service names, types, and type hashes: + * - Access to the array of names, types, and type hashes is not synchronized. * It is not safe to read or write `service_names_and_types` * while rmw_get_client_names_and_types_by_node() uses it. * - Access to C-style string arguments is read-only but it is not synchronized. @@ -282,7 +282,7 @@ rmw_get_service_names_and_types_by_node( * \param[in] allocator Allocator to be used when populating the `service_names_and_types` array. * \param[in] node_name Name of the remote node to get information for. * \param[in] node_namespace Namespace of the remote node to get information for. - * \param[out] service_names_and_types Array of service names and types the remote node has + * \param[out] service_names_and_types Array of service names, types, and type hashes the remote node has * created a service client for, populated on success but left unchanged on failure. * If populated, it is up to the caller to finalize this array later on * using rmw_names_and_types_fini(). diff --git a/rmw/include/rmw/get_service_names_and_types.h b/rmw/include/rmw/get_service_names_and_types.h index 5bf72491..ccb62450 100644 --- a/rmw/include/rmw/get_service_names_and_types.h +++ b/rmw/include/rmw/get_service_names_and_types.h @@ -25,9 +25,9 @@ extern "C" #include "rmw/types.h" #include "rmw/visibility_control.h" -/// Return all service names and types in the ROS graph. +/// Return all service names, types, and type hashes in the ROS graph. /** - * This function returns an array of all service names and types in the ROS graph + * This function returns an array of all service names, types, and type hashes in the ROS graph * i.e. for which a server and/or client exists, as discovered so far by the given * local node. * @@ -50,20 +50,20 @@ extern "C" * \par Thread-safety * Nodes are thread-safe objects, and so are all operations on them except for finalization. * Therefore, it is safe to query the ROS graph using the same node concurrently. - * However, when querying services names and types: - * - Access to the array of names and types is not synchronized. + * However, when querying services names, types, and type hashes: + * - Access to the array of names, types, and type hashes is not synchronized. * It is not safe to read or write `service_names_and_types` * while rmw_get_service_names_and_types() uses it. * - The default allocators are thread-safe objects, but any custom `allocator` may not be. * Check your allocator documentation for further reference. * * \pre Given `node` must be a valid node handle, as returned by rmw_create_node(). - * \pre Given `services_names_and_types` must be a zero-initialized array of names and types, + * \pre Given `services_names_and_types` must be a zero-initialized array of names, types, and type hashes, * as returned by rmw_get_zero_initialized_names_and_types(). * * \param[in] node Node to query the ROS graph. * \param[in] allocator Allocator to be used when populating the `service_names_and_types` array. - * \param[out] service_names_and_types Array of service names and their types, + * \param[out] service_names_and_types Array of service names, their types, and their type hashes, * populated on success but left unchanged on failure. * If populated, it is up to the caller to finalize this array later * on using rmw_names_and_types_fini(). diff --git a/rmw/include/rmw/get_topic_names_and_types.h b/rmw/include/rmw/get_topic_names_and_types.h index d070a351..2fa4b316 100644 --- a/rmw/include/rmw/get_topic_names_and_types.h +++ b/rmw/include/rmw/get_topic_names_and_types.h @@ -25,9 +25,9 @@ extern "C" #include "rmw/types.h" #include "rmw/visibility_control.h" -/// Return all topic names and types in the ROS graph. +/// Return all topic names, types, and type hashes in the ROS graph. /** - * This function returns an array of all topic names and types in the ROS graph + * This function returns an array of all topic names, types, and type hashes in the ROS graph * i.e. for which a publisher and/or a subscription exists, as discovered so far * by the given local node. * @@ -57,15 +57,15 @@ extern "C" * \par Thread-safety * Nodes are thread-safe objects, and so are all operations on them except for finalization. * Therefore, it is safe to query the ROS graph using the same node concurrently. - * However, when querying topic names and types: - * - Access to the array of names and types is not synchronized. + * However, when querying topic names, types, and type hashes: + * - Access to the array of names, types, and type hashes is not synchronized. * It is not safe to read or write `topic_names_and_types` * while rmw_get_topic_names_and_types() uses it. * - The default allocators are thread-safe objects, but any custom `allocator` may not be. * Check your allocator documentation for further reference. * * \pre Given `node` must be a valid node handle, as returned by rmw_create_node(). - * \pre Given `topic_names_and_types` must be a zero-initialized array of names and types, + * \pre Given `topic_names_and_types` must be a zero-initialized array of names, types, and type hashes, * as returned by rmw_get_zero_initialized_names_and_types(). * * \param[in] node Node to query the ROS graph. diff --git a/rmw/include/rmw/names_and_types.h b/rmw/include/rmw/names_and_types.h index 08280995..fa112e47 100644 --- a/rmw/include/rmw/names_and_types.h +++ b/rmw/include/rmw/names_and_types.h @@ -27,8 +27,9 @@ extern "C" #include "rmw/macros.h" #include "rmw/types.h" #include "rmw/visibility_control.h" +#include "rosidl_runtime_c/type_hash.h" -/// Associative array of topic or service names and types. +/// Associative array of topic or service names, types, and type hashes. typedef struct RMW_PUBLIC_TYPE rmw_names_and_types_s { /// Array of names @@ -36,9 +37,13 @@ typedef struct RMW_PUBLIC_TYPE rmw_names_and_types_s /// Dynamic array of arrays of type names, with the same length as `names` rcutils_string_array_t * types; + + /// Array of arrays of type hashes, with the same length as `names`. + /// `type_hashes[i][j]` corresponds to `types[i].data[j]`. + rosidl_type_hash_t ** type_hashes; } rmw_names_and_types_t; -/// Return a zero initialized array of names and types. +/// Return a zero initialized array of names, types, and type hashes. RMW_PUBLIC RMW_WARN_UNUSED rmw_names_and_types_t @@ -55,7 +60,7 @@ rmw_get_zero_initialized_names_and_types(void); * Lock-Free | Yes * * \par Thread-safety - * Access to the array of names and types is read-only, but it is not synchronized. + * Access to the array of names, types, and type hashes is read-only, but it is not synchronized. * Concurrent `names_and_types` reads are safe, but concurrent reads and writes are not. * * \param[in] names_and_types Array to be checked. @@ -67,7 +72,7 @@ RMW_WARN_UNUSED rmw_ret_t rmw_names_and_types_check_zero(rmw_names_and_types_t * names_and_types); -/// Initialize an array of names and types. +/// Initialize an array of names, types, and type hashes. /** * This function initializes the string array for the names and allocates space * for all the string arrays for the types according to the given size, but @@ -84,11 +89,14 @@ rmw_names_and_types_check_zero(rmw_names_and_types_t * names_and_types); * * \par Thread-safety * Initialization is a reentrant procedure, but: - * - Access to arrays of names and types is not synchronized. + * - Access to arrays of names, types, and type hashes is not synchronized. * It is not safe to read or write `names_and_types` during initialization. * - The default allocators are thread-safe objects, but any custom `allocator` may not be. * Check your allocator documentation for further reference. * + * \pre Given `names_and_types` must be a zero-initialized array, + * as returned by rmw_get_zero_initialized_names_and_types(). + * * \param[inout] names_and_types Array to be initialized on success, * but left unchanged on failure. * \param[in] size Size of the array. @@ -111,7 +119,7 @@ rmw_names_and_types_init( size_t size, rcutils_allocator_t * allocator); -/// Finalize an array of names and types. +/// Finalize an array of names, types, and type hashes. /** * This function deallocates the string array of names and the array of string arrays of types, * and zero initializes the given array. @@ -128,7 +136,7 @@ rmw_names_and_types_init( * Lock-Free | Yes * * \par Thread-safety - * Finalization is a reentrant procedure, but access to arrays of names and types + * Finalization is a reentrant procedure, but access to arrays of names, types, and type hashes * is not synchronized. * It is not safe to read or write `names_and_types` during initialization. * diff --git a/rmw/src/names_and_types.c b/rmw/src/names_and_types.c index 3c80d21f..82183132 100644 --- a/rmw/src/names_and_types.c +++ b/rmw/src/names_and_types.c @@ -46,6 +46,10 @@ rmw_names_and_types_check_zero(rmw_names_and_types_t * names_and_types) RMW_SET_ERROR_MSG("types array is not NULL"); return RMW_RET_INVALID_ARGUMENT; } + if (names_and_types->type_hashes != 0) { + RMW_SET_ERROR_MSG("type_hashes array is not NULL"); + return RMW_RET_INVALID_ARGUMENT; + } return RMW_RET_OK; } @@ -66,9 +70,16 @@ rmw_names_and_types_init( RMW_SET_ERROR_MSG("names_and_types is null"); return RMW_RET_INVALID_ARGUMENT; } + if (rmw_names_and_types_check_zero(names_and_types) != RMW_RET_OK) { + // rmw_names_and_types_check_zero already sets the error message + return RMW_RET_INVALID_ARGUMENT; + } rcutils_ret_t rcutils_ret = rcutils_string_array_init(&names_and_types->names, size, allocator); if (rcutils_ret != RCUTILS_RET_OK) { RMW_SET_ERROR_MSG(rcutils_get_error_string().str); + // rcutils_string_array_init may leave the array in a partial state on failure; + // restore the zero-initialized post-condition so the caller can re-init. + names_and_types->names = rcutils_get_zero_initialized_string_array(); return rmw_convert_rcutils_ret_to_rmw_ret(rcutils_ret); } names_and_types->types = @@ -81,6 +92,18 @@ rmw_names_and_types_init( RMW_SET_ERROR_MSG("failed to allocate memory for types"); return RMW_RET_BAD_ALLOC; } + names_and_types->type_hashes = + allocator->zero_allocate(size, sizeof(rosidl_type_hash_t *), allocator->state); + if (!names_and_types->type_hashes && size != 0) { + allocator->deallocate(names_and_types->types, allocator->state); + names_and_types->types = NULL; + rcutils_ret = rcutils_string_array_fini(&names_and_types->names); + if (rcutils_ret != RCUTILS_RET_OK) { + RCUTILS_LOG_ERROR("error while reporting error: %s", rcutils_get_error_string().str); + } + RMW_SET_ERROR_MSG("failed to allocate type_hashes array"); + return RMW_RET_BAD_ALLOC; + } return RMW_RET_OK; } @@ -92,12 +115,13 @@ rmw_names_and_types_fini(rmw_names_and_types_t * names_and_types) return RMW_RET_INVALID_ARGUMENT; } rcutils_ret_t rcutils_ret; - if (names_and_types->types) { + if (names_and_types->types || names_and_types->type_hashes || names_and_types->names.data) { RCUTILS_CHECK_ALLOCATOR_WITH_MSG( &names_and_types->names.allocator, "allocator is invalid", return RMW_RET_INVALID_ARGUMENT); - + } + if (names_and_types->types) { // Cleanup string arrays for types first size_t i; for (i = 0; i < names_and_types->names.size; ++i) { @@ -107,14 +131,30 @@ rmw_names_and_types_fini(rmw_names_and_types_t * names_and_types) return rmw_convert_rcutils_ret_to_rmw_ret(rcutils_ret); } } - // Use the allocator in the names string array // (prevents this data structure from having to also store it) names_and_types->names.allocator.deallocate( names_and_types->types, names_and_types->names.allocator.state); names_and_types->types = NULL; } - // Cleanup names string array + + // Cleanup type hashes array + if (names_and_types->type_hashes) { + size_t i; + for (i = 0; i < names_and_types->names.size; ++i) { + if (names_and_types->type_hashes[i]) { + names_and_types->names.allocator.deallocate( + names_and_types->type_hashes[i], names_and_types->names.allocator.state); + names_and_types->type_hashes[i] = NULL; + } + } + names_and_types->names.allocator.deallocate( + names_and_types->type_hashes, names_and_types->names.allocator.state); + names_and_types->type_hashes = NULL; + } + // Cleanup names string array. + // Safe on a zero-initialized names array; rcutils_string_array_fini is a + // no-op when data is NULL and does not touch the embedded allocator. rcutils_ret = rcutils_string_array_fini(&names_and_types->names); if (rcutils_ret != RCUTILS_RET_OK) { RMW_SET_ERROR_MSG(rcutils_get_error_string().str); diff --git a/rmw/test/test_names_and_types.cpp b/rmw/test/test_names_and_types.cpp index ae0b447d..6fb8c4c3 100644 --- a/rmw/test/test_names_and_types.cpp +++ b/rmw/test/test_names_and_types.cpp @@ -60,13 +60,20 @@ TEST(rmw_names_and_types, rmw_names_and_types_check_zero) { names_and_types.types = nullptr; rmw_reset_error(); + // type_hashes is not null + rosidl_type_hash_t * hash_ptr = reinterpret_cast(0x1); + names_and_types.type_hashes = &hash_ptr; + EXPECT_EQ(rmw_names_and_types_check_zero(&names_and_types), RMW_RET_INVALID_ARGUMENT); + names_and_types.type_hashes = nullptr; + rmw_reset_error(); + // OK names_and_types = rmw_get_zero_initialized_names_and_types(); EXPECT_EQ(rmw_names_and_types_check_zero(&names_and_types), RMW_RET_OK); } TEST(rmw_names_and_types, rmw_names_and_types_init) { - rmw_names_and_types_t names_and_types; + rmw_names_and_types_t names_and_types = rmw_get_zero_initialized_names_and_types(); rcutils_allocator_t allocator = rcutils_get_default_allocator(); size_t size = 100; @@ -78,6 +85,7 @@ TEST(rmw_names_and_types, rmw_names_and_types_init) { // allocator is not null, but still invalid rcutils_allocator_t invalid_allocator = get_time_bomb_allocator(); invalid_allocator.deallocate = nullptr; + names_and_types = rmw_get_zero_initialized_names_and_types(); result = rmw_names_and_types_init(&names_and_types, size, &invalid_allocator); EXPECT_EQ(result, RMW_RET_INVALID_ARGUMENT); rmw_reset_error(); @@ -90,15 +98,31 @@ TEST(rmw_names_and_types, rmw_names_and_types_init) { // allocator fails to allocate memory to names rcutils_allocator_t failing_allocator = get_time_bomb_allocator(); set_time_bomb_allocator_calloc_count(failing_allocator, 0); + names_and_types = rmw_get_zero_initialized_names_and_types(); result = rmw_names_and_types_init(&names_and_types, size, &failing_allocator); EXPECT_EQ(result, RMW_RET_BAD_ALLOC); rmw_reset_error(); + // After BAD_ALLOC the struct must be back to zero so the caller can re-init. + EXPECT_EQ(rmw_names_and_types_check_zero(&names_and_types), RMW_RET_OK); + rmw_reset_error(); // allocator fails to allocate memory to types set_time_bomb_allocator_calloc_count(failing_allocator, 1); + names_and_types = rmw_get_zero_initialized_names_and_types(); + result = rmw_names_and_types_init(&names_and_types, size, &failing_allocator); + EXPECT_EQ(result, RMW_RET_BAD_ALLOC); + rmw_reset_error(); + EXPECT_EQ(rmw_names_and_types_check_zero(&names_and_types), RMW_RET_OK); + rmw_reset_error(); + + // allocator fails to allocate memory to type_hashes + set_time_bomb_allocator_calloc_count(failing_allocator, 2); + names_and_types = rmw_get_zero_initialized_names_and_types(); result = rmw_names_and_types_init(&names_and_types, size, &failing_allocator); EXPECT_EQ(result, RMW_RET_BAD_ALLOC); rmw_reset_error(); + EXPECT_EQ(rmw_names_and_types_check_zero(&names_and_types), RMW_RET_OK); + rmw_reset_error(); // Fails to deallocate names after failing to zero allocate types set_time_bomb_allocator_calloc_count(failing_allocator, 1); @@ -111,6 +135,7 @@ TEST(rmw_names_and_types, rmw_names_and_types_init) { // If logging shutdown is not called, there's a small memory leak EXPECT_EQ(rcutils_logging_shutdown(), RCUTILS_RET_OK); }); + names_and_types = rmw_get_zero_initialized_names_and_types(); result = rmw_names_and_types_init(&names_and_types, size, &failing_allocator); EXPECT_EQ(result, RMW_RET_BAD_ALLOC); rmw_reset_error(); @@ -120,10 +145,12 @@ TEST(rmw_names_and_types, rmw_names_and_types_init) { ASSERT_EQ(rcutils_string_array_fini(&names_and_types.names), RMW_RET_OK); // Size == 0 is Ok + names_and_types = rmw_get_zero_initialized_names_and_types(); result = rmw_names_and_types_init(&names_and_types, 0, &allocator); EXPECT_EQ(result, RMW_RET_OK); EXPECT_EQ(rmw_names_and_types_fini(&names_and_types), RMW_RET_OK); + names_and_types = rmw_get_zero_initialized_names_and_types(); result = rmw_names_and_types_init(&names_and_types, size, &allocator); EXPECT_EQ(result, RMW_RET_OK); EXPECT_EQ(rmw_names_and_types_fini(&names_and_types), RMW_RET_OK);