From 33f66a20d43d301798accc2818472dacbe04bdfb Mon Sep 17 00:00:00 2001 From: Michael Carroll Date: Tue, 28 Aug 2018 16:48:51 -0500 Subject: [PATCH 1/5] Include node namespaces in get_node_names. --- rmw_fastrtps_cpp/src/rmw_node_names.cpp | 5 +- .../src/rmw_node_names.cpp | 5 +- .../custom_participant_info.hpp | 46 ++++++++++++++++--- .../rmw_fastrtps_shared_cpp/rmw_common.hpp | 3 +- rmw_fastrtps_shared_cpp/src/rmw_node.cpp | 15 +++--- .../src/rmw_node_names.cpp | 20 +++++++- 6 files changed, 71 insertions(+), 23 deletions(-) diff --git a/rmw_fastrtps_cpp/src/rmw_node_names.cpp b/rmw_fastrtps_cpp/src/rmw_node_names.cpp index 9db14f9e5..bcdbe6648 100644 --- a/rmw_fastrtps_cpp/src/rmw_node_names.cpp +++ b/rmw_fastrtps_cpp/src/rmw_node_names.cpp @@ -33,9 +33,10 @@ extern "C" rmw_ret_t rmw_get_node_names( const rmw_node_t * node, - rcutils_string_array_t * node_names) + rcutils_string_array_t * node_names, + rcutils_string_array_t * node_namespaces) { return rmw_fastrtps_shared_cpp::__rmw_get_node_names( - eprosima_fastrtps_identifier, node, node_names); + eprosima_fastrtps_identifier, node, node_names, node_namespaces); } } // extern "C" diff --git a/rmw_fastrtps_dynamic_cpp/src/rmw_node_names.cpp b/rmw_fastrtps_dynamic_cpp/src/rmw_node_names.cpp index f957ae4c6..55440d33a 100644 --- a/rmw_fastrtps_dynamic_cpp/src/rmw_node_names.cpp +++ b/rmw_fastrtps_dynamic_cpp/src/rmw_node_names.cpp @@ -33,9 +33,10 @@ extern "C" rmw_ret_t rmw_get_node_names( const rmw_node_t * node, - rcutils_string_array_t * node_names) + rcutils_string_array_t * node_names, + rcutils_string_array_t * node_namespaces) { return rmw_fastrtps_shared_cpp::__rmw_get_node_names( - eprosima_fastrtps_identifier, node, node_names); + eprosima_fastrtps_identifier, node, node_names, node_namespaces); } } // extern "C" diff --git a/rmw_fastrtps_shared_cpp/include/rmw_fastrtps_shared_cpp/custom_participant_info.hpp b/rmw_fastrtps_shared_cpp/include/rmw_fastrtps_shared_cpp/custom_participant_info.hpp index 775739875..9c8702967 100644 --- a/rmw_fastrtps_shared_cpp/include/rmw_fastrtps_shared_cpp/custom_participant_info.hpp +++ b/rmw_fastrtps_shared_cpp/include/rmw_fastrtps_shared_cpp/custom_participant_info.hpp @@ -58,11 +58,21 @@ class ParticipantListener : public eprosima::fastrtps::ParticipantListener // ignore already known GUIDs if (discovered_names.find(info.rtps.m_guid) == discovered_names.end()) { auto map = rmw::impl::cpp::parse_key_value(info.rtps.m_userData); - auto found = map.find("name"); + auto name_found = map.find("name"); + auto ns_found = map.find("namespace"); + std::string name; - if (found != map.end()) { - name = std::string(found->second.begin(), found->second.end()); + if (name_found != map.end()) { + name = std::string(name_found->second.begin(), name_found->second.end()); + } + + std::string namespace_; + if (ns_found != map.end()) { + namespace_ = std::string(ns_found->second.begin(), ns_found->second.end()); } + + fprintf(stderr, "%s %s\n", name.c_str(), namespace_.c_str()); + if (name.empty()) { // use participant name if no name was found in the user data name = info.rtps.m_RTPSParticipantName; @@ -70,13 +80,23 @@ class ParticipantListener : public eprosima::fastrtps::ParticipantListener // ignore discovered participants without a name if (!name.empty()) { discovered_names[info.rtps.m_guid] = name; + discovered_namespaces[info.rtps.m_guid] = namespace_; } } } else { - auto it = discovered_names.find(info.rtps.m_guid); - // only consider known GUIDs - if (it != discovered_names.end()) { - discovered_names.erase(it); + { + auto it = discovered_names.find(info.rtps.m_guid); + // only consider known GUIDs + if (it != discovered_names.end()) { + discovered_names.erase(it); + } + } + { + auto it = discovered_namespaces.find(info.rtps.m_guid); + // only consider known GUIDs + if (it != discovered_namespaces.end()) { + discovered_namespaces.erase(it); + } } } } @@ -91,7 +111,19 @@ class ParticipantListener : public eprosima::fastrtps::ParticipantListener return names; } + std::vector get_discovered_namespaces() const + { + std::vector namespaces(discovered_namespaces.size()); + size_t i = 0; + for (auto it : discovered_namespaces) { + namespaces[i++] = it.second; + } + return namespaces; + } + + std::map discovered_names; + std::map discovered_namespaces; }; #endif // RMW_FASTRTPS_SHARED_CPP__CUSTOM_PARTICIPANT_INFO_HPP_ diff --git a/rmw_fastrtps_shared_cpp/include/rmw_fastrtps_shared_cpp/rmw_common.hpp b/rmw_fastrtps_shared_cpp/include/rmw_fastrtps_shared_cpp/rmw_common.hpp index 59d6e90c1..44c06a77e 100644 --- a/rmw_fastrtps_shared_cpp/include/rmw_fastrtps_shared_cpp/rmw_common.hpp +++ b/rmw_fastrtps_shared_cpp/include/rmw_fastrtps_shared_cpp/rmw_common.hpp @@ -105,7 +105,8 @@ rmw_ret_t __rmw_get_node_names( const char * identifier, const rmw_node_t * node, - rcutils_string_array_t * node_names); + rcutils_string_array_t * node_names, + rcutils_string_array_t * node_namespaces); RMW_FASTRTPS_SHARED_CPP_PUBLIC rmw_ret_t diff --git a/rmw_fastrtps_shared_cpp/src/rmw_node.cpp b/rmw_fastrtps_shared_cpp/src/rmw_node.cpp index 6d06ef1e0..38272ca1c 100644 --- a/rmw_fastrtps_shared_cpp/src/rmw_node.cpp +++ b/rmw_fastrtps_shared_cpp/src/rmw_node.cpp @@ -251,15 +251,12 @@ __rmw_create_node( eprosima::fastrtps::rtps::PREALLOCATED_WITH_REALLOC_MEMORY_MODE; participantAttrs.rtps.builtin.writerHistoryMemoryPolicy = eprosima::fastrtps::rtps::PREALLOCATED_WITH_REALLOC_MEMORY_MODE; - // the node name is also set in the user_data - size_t name_length = strlen(name); - const char prefix[6] = "name="; - participantAttrs.rtps.userData.resize(name_length + sizeof(prefix)); - memcpy(participantAttrs.rtps.userData.data(), prefix, sizeof(prefix) - 1); - for (size_t i = 0; i < name_length; ++i) { - participantAttrs.rtps.userData[sizeof(prefix) - 1 + i] = name[i]; - } - participantAttrs.rtps.userData[sizeof(prefix) - 1 + name_length] = ';'; + + size_t length = strlen(name) + strlen("name=;") + + strlen(namespace_) + strlen("namespace=;") + 1; + participantAttrs.rtps.userData.resize(length); + snprintf(reinterpret_cast(participantAttrs.rtps.userData.data()), + length, "name=%s;namespace=%s;", name, namespace_); if (security_options->security_root_path) { // if security_root_path provided, try to find the key and certificate files diff --git a/rmw_fastrtps_shared_cpp/src/rmw_node_names.cpp b/rmw_fastrtps_shared_cpp/src/rmw_node_names.cpp index 9bb5d64c2..8bd5802cf 100644 --- a/rmw_fastrtps_shared_cpp/src/rmw_node_names.cpp +++ b/rmw_fastrtps_shared_cpp/src/rmw_node_names.cpp @@ -38,7 +38,8 @@ rmw_ret_t __rmw_get_node_names( const char * identifier, const rmw_node_t * node, - rcutils_string_array_t * node_names) + rcutils_string_array_t * node_names, + rcutils_string_array_t * node_namespaces) { if (!node) { RMW_SET_ERROR_MSG("null node handle"); @@ -47,6 +48,9 @@ __rmw_get_node_names( if (rmw_check_zero_rmw_string_array(node_names) != RMW_RET_OK) { return RMW_RET_ERROR; } + if (rmw_check_zero_rmw_string_array(node_namespaces) != RMW_RET_OK) { + return RMW_RET_ERROR; + } // Get participant pointer from node if (node->implementation_identifier != identifier) { @@ -56,6 +60,7 @@ __rmw_get_node_names( auto impl = static_cast(node->data); auto participant_names = impl->listener->get_discovered_names(); + auto participant_ns = impl->listener->get_discovered_namespaces(); rcutils_allocator_t allocator = rcutils_get_default_allocator(); rcutils_ret_t rcutils_ret = @@ -64,15 +69,26 @@ __rmw_get_node_names( RMW_SET_ERROR_MSG(rcutils_get_error_string_safe()) return rmw_convert_rcutils_ret_to_rmw_ret(rcutils_ret); } + + rcutils_ret = + rcutils_string_array_init(node_namespaces, participant_names.size() + 1, &allocator); + if (rcutils_ret != RCUTILS_RET_OK) { + RMW_SET_ERROR_MSG(rcutils_get_error_string_safe()) + return rmw_convert_rcutils_ret_to_rmw_ret(rcutils_ret); + } + for (size_t i = 0; i < participant_names.size() + 1; ++i) { if (0 == i) { node_names->data[i] = rcutils_strdup(node->name, allocator); + node_namespaces->data[i] = rcutils_strdup(node->namespace_, allocator); } else { node_names->data[i] = rcutils_strdup(participant_names[i - 1].c_str(), allocator); + node_namespaces->data[i] = rcutils_strdup(participant_ns[i - 1].c_str(), allocator); } - if (!node_names->data[i]) { + if (!node_names->data[i] || !node_namespaces->data[i]) { RMW_SET_ERROR_MSG("failed to allocate memory for node name") rcutils_ret = rcutils_string_array_fini(node_names); + rcutils_ret = rcutils_string_array_fini(node_namespaces); if (rcutils_ret != RCUTILS_RET_OK) { RCUTILS_LOG_ERROR_NAMED( "rmw_fastrtps_shared_cpp", From 81cb66f04a8e2c59d6cb10cad69bc66e82abb208 Mon Sep 17 00:00:00 2001 From: Michael Carroll Date: Wed, 29 Aug 2018 09:57:29 -0500 Subject: [PATCH 2/5] Remove debug printf. --- .../include/rmw_fastrtps_shared_cpp/custom_participant_info.hpp | 2 -- 1 file changed, 2 deletions(-) diff --git a/rmw_fastrtps_shared_cpp/include/rmw_fastrtps_shared_cpp/custom_participant_info.hpp b/rmw_fastrtps_shared_cpp/include/rmw_fastrtps_shared_cpp/custom_participant_info.hpp index 9c8702967..4e086435d 100644 --- a/rmw_fastrtps_shared_cpp/include/rmw_fastrtps_shared_cpp/custom_participant_info.hpp +++ b/rmw_fastrtps_shared_cpp/include/rmw_fastrtps_shared_cpp/custom_participant_info.hpp @@ -71,8 +71,6 @@ class ParticipantListener : public eprosima::fastrtps::ParticipantListener namespace_ = std::string(ns_found->second.begin(), ns_found->second.end()); } - fprintf(stderr, "%s %s\n", name.c_str(), namespace_.c_str()); - if (name.empty()) { // use participant name if no name was found in the user data name = info.rtps.m_RTPSParticipantName; From 88e906bbc1f7dfdf3b650052b9f3f70297b9332e Mon Sep 17 00:00:00 2001 From: Michael Carroll Date: Thu, 30 Aug 2018 10:33:47 -0500 Subject: [PATCH 3/5] Address reviewer feedback. * Check snprintf return * Better error handling on get_node_names. --- rmw_fastrtps_shared_cpp/src/rmw_node.cpp | 8 +++-- .../src/rmw_node_names.cpp | 33 +++++++++++++------ 2 files changed, 29 insertions(+), 12 deletions(-) diff --git a/rmw_fastrtps_shared_cpp/src/rmw_node.cpp b/rmw_fastrtps_shared_cpp/src/rmw_node.cpp index 38272ca1c..510ab30f7 100644 --- a/rmw_fastrtps_shared_cpp/src/rmw_node.cpp +++ b/rmw_fastrtps_shared_cpp/src/rmw_node.cpp @@ -255,8 +255,12 @@ __rmw_create_node( size_t length = strlen(name) + strlen("name=;") + strlen(namespace_) + strlen("namespace=;") + 1; participantAttrs.rtps.userData.resize(length); - snprintf(reinterpret_cast(participantAttrs.rtps.userData.data()), - length, "name=%s;namespace=%s;", name, namespace_); + size_t written = snprintf(reinterpret_cast(participantAttrs.rtps.userData.data()), + length, "name=%s;namespace=%s;", name, namespace_); + if (written < 0) { + RMW_SET_ERROR_MSG("failed to populate user_data buffer"); + return nullptr; + } if (security_options->security_root_path) { // if security_root_path provided, try to find the key and certificate files diff --git a/rmw_fastrtps_shared_cpp/src/rmw_node_names.cpp b/rmw_fastrtps_shared_cpp/src/rmw_node_names.cpp index 8bd5802cf..ab4d6b778 100644 --- a/rmw_fastrtps_shared_cpp/src/rmw_node_names.cpp +++ b/rmw_fastrtps_shared_cpp/src/rmw_node_names.cpp @@ -67,14 +67,14 @@ __rmw_get_node_names( rcutils_string_array_init(node_names, participant_names.size() + 1, &allocator); if (rcutils_ret != RCUTILS_RET_OK) { RMW_SET_ERROR_MSG(rcutils_get_error_string_safe()) - return rmw_convert_rcutils_ret_to_rmw_ret(rcutils_ret); + goto fail; } rcutils_ret = rcutils_string_array_init(node_namespaces, participant_names.size() + 1, &allocator); if (rcutils_ret != RCUTILS_RET_OK) { RMW_SET_ERROR_MSG(rcutils_get_error_string_safe()) - return rmw_convert_rcutils_ret_to_rmw_ret(rcutils_ret); + goto fail; } for (size_t i = 0; i < participant_names.size() + 1; ++i) { @@ -87,16 +87,29 @@ __rmw_get_node_names( } if (!node_names->data[i] || !node_namespaces->data[i]) { RMW_SET_ERROR_MSG("failed to allocate memory for node name") - rcutils_ret = rcutils_string_array_fini(node_names); - rcutils_ret = rcutils_string_array_fini(node_namespaces); - if (rcutils_ret != RCUTILS_RET_OK) { - RCUTILS_LOG_ERROR_NAMED( - "rmw_fastrtps_shared_cpp", - "failed to cleanup during error handling: %s", rcutils_get_error_string_safe()) - } - return RMW_RET_BAD_ALLOC; + goto fail; } } return RMW_RET_OK; +fail: + if (node_names) { + rcutils_ret = rcutils_string_array_fini(node_names); + if (rcutils_ret != RCUTILS_RET_OK) { + RCUTILS_LOG_ERROR_NAMED( + "rmw_connext_cpp", + "failed to cleanup during error handling: %s", rcutils_get_error_string_safe()) + rcutils_reset_error(); + } + } + if (node_namesspaces) { + rcutils_ret = rcutils_string_array_fini(node_namespaces); + if (rcutils_ret != RCUTILS_RET_OK) { + RCUTILS_LOG_ERROR_NAMED( + "rmw_connext_cpp", + "failed to cleanup during error handling: %s", rcutils_get_error_string_safe()) + rcutils_reset_error(); + } + } + return RMW_RET_BAD_ALLOC; } } // namespace rmw_fastrtps_shared_cpp From e6b8627fa1b4b8208c1f43fca04d6378a169c30b Mon Sep 17 00:00:00 2001 From: Michael Carroll Date: Tue, 4 Sep 2018 15:09:49 -0500 Subject: [PATCH 4/5] Check snprintf output. --- rmw_fastrtps_shared_cpp/src/rmw_node.cpp | 4 ++-- rmw_fastrtps_shared_cpp/src/rmw_node_names.cpp | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/rmw_fastrtps_shared_cpp/src/rmw_node.cpp b/rmw_fastrtps_shared_cpp/src/rmw_node.cpp index 510ab30f7..3f50b01ed 100644 --- a/rmw_fastrtps_shared_cpp/src/rmw_node.cpp +++ b/rmw_fastrtps_shared_cpp/src/rmw_node.cpp @@ -255,9 +255,9 @@ __rmw_create_node( size_t length = strlen(name) + strlen("name=;") + strlen(namespace_) + strlen("namespace=;") + 1; participantAttrs.rtps.userData.resize(length); - size_t written = snprintf(reinterpret_cast(participantAttrs.rtps.userData.data()), + int written = snprintf(reinterpret_cast(participantAttrs.rtps.userData.data()), length, "name=%s;namespace=%s;", name, namespace_); - if (written < 0) { + if (written < 0 || written > length - 1) { RMW_SET_ERROR_MSG("failed to populate user_data buffer"); return nullptr; } diff --git a/rmw_fastrtps_shared_cpp/src/rmw_node_names.cpp b/rmw_fastrtps_shared_cpp/src/rmw_node_names.cpp index ab4d6b778..6008d86bc 100644 --- a/rmw_fastrtps_shared_cpp/src/rmw_node_names.cpp +++ b/rmw_fastrtps_shared_cpp/src/rmw_node_names.cpp @@ -101,7 +101,7 @@ __rmw_get_node_names( rcutils_reset_error(); } } - if (node_namesspaces) { + if (node_namespaces) { rcutils_ret = rcutils_string_array_fini(node_namespaces); if (rcutils_ret != RCUTILS_RET_OK) { RCUTILS_LOG_ERROR_NAMED( From 435dfb7d1b6a1af72a0fd86407c9bec89e78797b Mon Sep 17 00:00:00 2001 From: Michael Carroll Date: Thu, 6 Sep 2018 08:01:45 -0500 Subject: [PATCH 5/5] Fix compiler warning. --- rmw_fastrtps_shared_cpp/src/rmw_node.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rmw_fastrtps_shared_cpp/src/rmw_node.cpp b/rmw_fastrtps_shared_cpp/src/rmw_node.cpp index 3f50b01ed..83eec1bd1 100644 --- a/rmw_fastrtps_shared_cpp/src/rmw_node.cpp +++ b/rmw_fastrtps_shared_cpp/src/rmw_node.cpp @@ -257,7 +257,7 @@ __rmw_create_node( participantAttrs.rtps.userData.resize(length); int written = snprintf(reinterpret_cast(participantAttrs.rtps.userData.data()), length, "name=%s;namespace=%s;", name, namespace_); - if (written < 0 || written > length - 1) { + if (written < 0 || written > static_cast(length) - 1) { RMW_SET_ERROR_MSG("failed to populate user_data buffer"); return nullptr; }