From 2f2d195def9a4fc33f9ed561c14c8cec667a9a06 Mon Sep 17 00:00:00 2001 From: Michel Hidalgo Date: Mon, 6 Jul 2020 17:13:54 -0300 Subject: [PATCH 1/7] Ensure compliant node construction/destruction API. Signed-off-by: Michel Hidalgo --- rmw_fastrtps_cpp/src/rmw_node.cpp | 19 +++- rmw_fastrtps_dynamic_cpp/src/rmw_node.cpp | 20 +++- rmw_fastrtps_shared_cpp/src/rmw_node.cpp | 126 +++++++++++----------- 3 files changed, 102 insertions(+), 63 deletions(-) diff --git a/rmw_fastrtps_cpp/src/rmw_node.cpp b/rmw_fastrtps_cpp/src/rmw_node.cpp index 5f1daf5c1..2652965e5 100644 --- a/rmw_fastrtps_cpp/src/rmw_node.cpp +++ b/rmw_fastrtps_cpp/src/rmw_node.cpp @@ -28,6 +28,7 @@ #include "rmw_fastrtps_shared_cpp/init_rmw_context_impl.hpp" #include "rmw_fastrtps_shared_cpp/rmw_common.hpp" +#include "rmw_fastrtps_shared_cpp/rmw_context_impl.hpp" #include "rmw_fastrtps_cpp/identifier.hpp" #include "rmw_fastrtps_cpp/init_rmw_context_impl.hpp" @@ -40,13 +41,21 @@ rmw_create_node( const char * name, const char * namespace_) { - RCUTILS_CHECK_ARGUMENT_FOR_NULL(context, NULL); + RMW_CHECK_ARGUMENT_FOR_NULL(context, nullptr); RMW_CHECK_TYPE_IDENTIFIERS_MATCH( init context, context->implementation_identifier, eprosima_fastrtps_identifier, // TODO(wjwwood): replace this with RMW_RET_INCORRECT_RMW_IMPLEMENTATION when refactored return nullptr); + RMW_CHECK_FOR_NULL_WITH_MSG( + context->impl, + "expected initialized context", + return nullptr); + if (context->impl->is_shutdown) { + RCUTILS_SET_ERROR_MSG("context has been shutdown"); + return nullptr; + } if (RMW_RET_OK != rmw_fastrtps_cpp::increment_context_impl_ref_count(context)) { return nullptr; @@ -68,6 +77,14 @@ rmw_create_node( rmw_ret_t rmw_destroy_node(rmw_node_t * node) { + RMW_CHECK_ARGUMENT_FOR_NULL(node, RMW_RET_INVALID_ARGUMENT); + RMW_CHECK_TYPE_IDENTIFIERS_MATCH( + node, + node->implementation_identifier, + eprosima_fastrtps_identifier, + return RMW_RET_INCORRECT_RMW_IMPLEMENTATION); + assert(node->context != nullptr); + rmw_context_t * context = node->context; rmw_ret_t ret = rmw_fastrtps_shared_cpp::__rmw_destroy_node( eprosima_fastrtps_identifier, node); diff --git a/rmw_fastrtps_dynamic_cpp/src/rmw_node.cpp b/rmw_fastrtps_dynamic_cpp/src/rmw_node.cpp index d6f345fc1..94f914f5d 100644 --- a/rmw_fastrtps_dynamic_cpp/src/rmw_node.cpp +++ b/rmw_fastrtps_dynamic_cpp/src/rmw_node.cpp @@ -28,6 +28,7 @@ #include "rmw_fastrtps_shared_cpp/init_rmw_context_impl.hpp" #include "rmw_fastrtps_shared_cpp/rmw_common.hpp" +#include "rmw_fastrtps_shared_cpp/rmw_context_impl.hpp" #include "rmw_fastrtps_dynamic_cpp/identifier.hpp" #include "rmw_fastrtps_dynamic_cpp/init_rmw_context_impl.hpp" @@ -40,13 +41,21 @@ rmw_create_node( const char * name, const char * namespace_) { - RCUTILS_CHECK_ARGUMENT_FOR_NULL(context, NULL); + RMW_CHECK_ARGUMENT_FOR_NULL(context, nullptr); RMW_CHECK_TYPE_IDENTIFIERS_MATCH( init context, context->implementation_identifier, eprosima_fastrtps_identifier, // TODO(wjwwood): replace this with RMW_RET_INCORRECT_RMW_IMPLEMENTATION when refactored - return NULL); + return nullptr); + RMW_CHECK_FOR_NULL_WITH_MSG( + context->impl, + "expected initialized context", + return nullptr); + if (context->impl->is_shutdown) { + RCUTILS_SET_ERROR_MSG("context has been shutdown"); + return nullptr; + } if (RMW_RET_OK != rmw_fastrtps_dynamic_cpp::increment_context_impl_ref_count(context)) { return nullptr; @@ -68,6 +77,13 @@ rmw_create_node( rmw_ret_t rmw_destroy_node(rmw_node_t * node) { + RMW_CHECK_ARGUMENT_FOR_NULL(node, RMW_RET_INVALID_ARGUMENT); + RMW_CHECK_TYPE_IDENTIFIERS_MATCH( + node, + node->implementation_identifier, + eprosima_fastrtps_identifier, + return RMW_RET_INCORRECT_RMW_IMPLEMENTATION); + assert(node->context != nullptr); rmw_context_t * context = node->context; rmw_ret_t ret = rmw_fastrtps_shared_cpp::__rmw_destroy_node( eprosima_fastrtps_identifier, node); diff --git a/rmw_fastrtps_shared_cpp/src/rmw_node.cpp b/rmw_fastrtps_shared_cpp/src/rmw_node.cpp index 14c935102..07c04c443 100644 --- a/rmw_fastrtps_shared_cpp/src/rmw_node.cpp +++ b/rmw_fastrtps_shared_cpp/src/rmw_node.cpp @@ -20,11 +20,16 @@ #include #include "rcutils/logging_macros.h" +#include "rcutils/strdup.h" #include "rmw/allocators.h" #include "rmw/error_handling.h" #include "rmw/impl/cpp/macros.hpp" #include "rmw/rmw.h" +#include "rmw/validate_namespace.h" +#include "rmw/validate_node_name.h" + +#include "rcpputils/scope_exit.hpp" #include "rmw_dds_common/context.hpp" @@ -42,45 +47,60 @@ __rmw_create_node( const char * name, const char * namespace_) { - if (!name) { - RMW_SET_ERROR_MSG("name is null"); + assert(nullptr != context); + assert(nullptr != context->impl); + assert(identifier == context->implementation_identifier); + static_cast(identifier); + + int validation_result = -1; + rmw_ret_t ret = rmw_validate_node_name(name, &validation_result, nullptr); + if (RMW_RET_OK != ret || RMW_NODE_NAME_VALID != validation_result) { + const char * reason = RMW_RET_OK == ret ? + rmw_node_name_validation_result_string(validation_result) : + rmw_get_error_string().str; + RMW_SET_ERROR_MSG_WITH_FORMAT_STRING("invalid node name: %s", reason); return nullptr; } - - if (!namespace_) { - RMW_SET_ERROR_MSG("namespace_ is null"); + validation_result = -1; + ret = rmw_validate_namespace(namespace_, &validation_result, nullptr); + if (RMW_RET_OK != ret || RMW_NAMESPACE_VALID != validation_result) { + const char * reason = RMW_RET_OK == ret ? + rmw_node_name_validation_result_string(validation_result) : + rmw_get_error_string().str; + RMW_SET_ERROR_MSG_WITH_FORMAT_STRING("invalid node namespace: %s", reason); return nullptr; } - rmw_node_t * node_handle = nullptr; auto common_context = static_cast(context->impl->common); rmw_dds_common::GraphCache & graph_cache = common_context->graph_cache; - - node_handle = rmw_node_allocate(); - if (!node_handle) { - RMW_SET_ERROR_MSG("failed to allocate rmw_node_t"); - goto fail; + rcutils_allocator_t allocator = context->options.allocator; + rmw_node_t * node = reinterpret_cast( + allocator.zero_allocate(1u, sizeof(rmw_node_t), allocator.state)); + if (nullptr == node) { + RMW_SET_ERROR_MSG("failed to allocate node"); + return nullptr; } - node_handle->implementation_identifier = identifier; - node_handle->data = nullptr; - - node_handle->name = - static_cast(rmw_allocate(sizeof(char) * strlen(name) + 1)); - if (!node_handle->name) { - RMW_SET_ERROR_MSG("failed to allocate memory"); - node_handle->namespace_ = nullptr; // to avoid free on uninitialized memory - goto fail; + auto cleanup_node = rcpputils::make_scope_exit( + [node, allocator]() { + allocator.deallocate(const_cast(node->name), allocator.state); + allocator.deallocate(const_cast(node->namespace_), allocator.state); + allocator.deallocate(node, allocator.state); + }); + node->implementation_identifier = context->implementation_identifier; + node->data = nullptr; + + node->name = rcutils_strdup(name, allocator); + if (nullptr == node->name) { + RMW_SET_ERROR_MSG("failed to copy node name"); + return nullptr; } - memcpy(const_cast(node_handle->name), name, strlen(name) + 1); - node_handle->namespace_ = - static_cast(rmw_allocate(sizeof(char) * strlen(namespace_) + 1)); - if (!node_handle->namespace_) { - RMW_SET_ERROR_MSG("failed to allocate memory"); - goto fail; + node->namespace_ = rcutils_strdup(namespace_, allocator); + if (nullptr == node->namespace_) { + RMW_SET_ERROR_MSG("failed to copy node namespace"); + return nullptr; } - memcpy(const_cast(node_handle->namespace_), namespace_, strlen(namespace_) + 1); - node_handle->context = context; + node->context = context; { // Though graph_cache methods are thread safe, both cache update and publishing have to also @@ -92,24 +112,16 @@ __rmw_create_node( rmw_dds_common::msg::ParticipantEntitiesInfo participant_msg = graph_cache.add_node(common_context->gid, name, namespace_); if (RMW_RET_OK != __rmw_publish( - identifier, + node->implementation_identifier, common_context->pub, static_cast(&participant_msg), nullptr)) { - goto fail; + return nullptr; } } - return node_handle; -fail: - if (node_handle) { - rmw_free(const_cast(node_handle->namespace_)); - node_handle->namespace_ = nullptr; - rmw_free(const_cast(node_handle->name)); - node_handle->name = nullptr; - } - rmw_node_free(node_handle); - return nullptr; + cleanup_node.cancel(); + return node; } rmw_ret_t @@ -117,38 +129,32 @@ __rmw_destroy_node( const char * identifier, rmw_node_t * node) { - rmw_ret_t result_ret = RMW_RET_OK; - if (!node) { - RMW_SET_ERROR_MSG("node handle is null"); - return RMW_RET_ERROR; - } - - if (node->implementation_identifier != identifier) { - RMW_SET_ERROR_MSG("node handle not from this implementation"); - return RMW_RET_ERROR; - } + assert(node != nullptr); + assert(node->implementation_identifier == identifier); + static_cast(identifier); + assert(node->context != nullptr); + assert(node->context->impl != nullptr); + assert(node->context->impl->common != nullptr); auto common_context = static_cast(node->context->impl->common); rmw_dds_common::GraphCache & graph_cache = common_context->graph_cache; { std::lock_guard guard(common_context->node_update_mutex); rmw_dds_common::msg::ParticipantEntitiesInfo participant_msg = graph_cache.remove_node(common_context->gid, node->name, node->namespace_); - result_ret = __rmw_publish( - identifier, + rmw_ret_t ret = __rmw_publish( + node->implementation_identifier, common_context->pub, static_cast(&participant_msg), nullptr); - if (RMW_RET_OK != result_ret) { - return result_ret; + if (RMW_RET_OK != ret) { + return ret; } } - rmw_free(const_cast(node->name)); - node->name = nullptr; - rmw_free(const_cast(node->namespace_)); - node->namespace_ = nullptr; - rmw_node_free(node); - + rcutils_allocator_t allocator = node->context->options.allocator; + allocator.deallocate(const_cast(node->name), allocator.state); + allocator.deallocate(const_cast(node->namespace_), allocator.state); + allocator.deallocate(node, allocator.state); return RMW_RET_OK; } From 85a8b45d37061e7b64a3a47143fb1dc3cd28cbd4 Mon Sep 17 00:00:00 2001 From: Michel Hidalgo Date: Mon, 13 Jul 2020 16:45:55 -0300 Subject: [PATCH 2/7] Use known validation result values. Signed-off-by: Michel Hidalgo --- rmw_fastrtps_shared_cpp/src/rmw_node.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/rmw_fastrtps_shared_cpp/src/rmw_node.cpp b/rmw_fastrtps_shared_cpp/src/rmw_node.cpp index 07c04c443..3fdbab963 100644 --- a/rmw_fastrtps_shared_cpp/src/rmw_node.cpp +++ b/rmw_fastrtps_shared_cpp/src/rmw_node.cpp @@ -52,7 +52,7 @@ __rmw_create_node( assert(identifier == context->implementation_identifier); static_cast(identifier); - int validation_result = -1; + int validation_result = RMW_NODE_NAME_VALID; rmw_ret_t ret = rmw_validate_node_name(name, &validation_result, nullptr); if (RMW_RET_OK != ret || RMW_NODE_NAME_VALID != validation_result) { const char * reason = RMW_RET_OK == ret ? @@ -61,7 +61,7 @@ __rmw_create_node( RMW_SET_ERROR_MSG_WITH_FORMAT_STRING("invalid node name: %s", reason); return nullptr; } - validation_result = -1; + validation_result = RMW_NAMESPACE_VALID; ret = rmw_validate_namespace(namespace_, &validation_result, nullptr); if (RMW_RET_OK != ret || RMW_NAMESPACE_VALID != validation_result) { const char * reason = RMW_RET_OK == ret ? From 3921f1c0ce1bbab3083e16068add30b9faac1cb2 Mon Sep 17 00:00:00 2001 From: Michel Hidalgo Date: Mon, 13 Jul 2020 16:53:47 -0300 Subject: [PATCH 3/7] Revert allocator use changes. Signed-off-by: Michel Hidalgo --- rmw_fastrtps_shared_cpp/src/rmw_node.cpp | 44 +++++++++++++----------- 1 file changed, 23 insertions(+), 21 deletions(-) diff --git a/rmw_fastrtps_shared_cpp/src/rmw_node.cpp b/rmw_fastrtps_shared_cpp/src/rmw_node.cpp index 3fdbab963..a21c8314d 100644 --- a/rmw_fastrtps_shared_cpp/src/rmw_node.cpp +++ b/rmw_fastrtps_shared_cpp/src/rmw_node.cpp @@ -20,7 +20,6 @@ #include #include "rcutils/logging_macros.h" -#include "rcutils/strdup.h" #include "rmw/allocators.h" #include "rmw/error_handling.h" @@ -74,33 +73,37 @@ __rmw_create_node( auto common_context = static_cast(context->impl->common); rmw_dds_common::GraphCache & graph_cache = common_context->graph_cache; rcutils_allocator_t allocator = context->options.allocator; - rmw_node_t * node = reinterpret_cast( - allocator.zero_allocate(1u, sizeof(rmw_node_t), allocator.state)); - if (nullptr == node) { + rmw_node_t * node_handle = rmw_node_allocate(); + if (nullptr == node_handle) { RMW_SET_ERROR_MSG("failed to allocate node"); return nullptr; } auto cleanup_node = rcpputils::make_scope_exit( - [node, allocator]() { - allocator.deallocate(const_cast(node->name), allocator.state); - allocator.deallocate(const_cast(node->namespace_), allocator.state); - allocator.deallocate(node, allocator.state); + [node_handle, allocator]() { + rmw_free(const_cast(node_handle->name)); + rmw_free(const_cast(node_handle->namespace_)); + rmw_node_free(node_handle); }); - node->implementation_identifier = context->implementation_identifier; - node->data = nullptr; + node_handle->implementation_identifier = context->implementation_identifier; + node_handle->data = nullptr; - node->name = rcutils_strdup(name, allocator); - if (nullptr == node->name) { + node_handle->name = + static_cast(rmw_allocate(sizeof(char) * strlen(name) + 1)); + if (nullptr == node_handle->name) { RMW_SET_ERROR_MSG("failed to copy node name"); return nullptr; } + memcpy(const_cast(node_handle->name), name, strlen(name) + 1); - node->namespace_ = rcutils_strdup(namespace_, allocator); - if (nullptr == node->namespace_) { + node_handle->namespace_ = + static_cast(rmw_allocate(sizeof(char) * strlen(namespace_) + 1)); + if (nullptr == node_handle->namespace_) { RMW_SET_ERROR_MSG("failed to copy node namespace"); return nullptr; } - node->context = context; + memcpy(const_cast(node_handle->namespace_), namespace_, strlen(namespace_) + 1); + + node_handle->context = context; { // Though graph_cache methods are thread safe, both cache update and publishing have to also @@ -112,7 +115,7 @@ __rmw_create_node( rmw_dds_common::msg::ParticipantEntitiesInfo participant_msg = graph_cache.add_node(common_context->gid, name, namespace_); if (RMW_RET_OK != __rmw_publish( - node->implementation_identifier, + node_handle->implementation_identifier, common_context->pub, static_cast(&participant_msg), nullptr)) @@ -121,7 +124,7 @@ __rmw_create_node( } } cleanup_node.cancel(); - return node; + return node_handle; } rmw_ret_t @@ -151,10 +154,9 @@ __rmw_destroy_node( return ret; } } - rcutils_allocator_t allocator = node->context->options.allocator; - allocator.deallocate(const_cast(node->name), allocator.state); - allocator.deallocate(const_cast(node->namespace_), allocator.state); - allocator.deallocate(node, allocator.state); + rmw_free(const_cast(node->name)); + rmw_free(const_cast(node->namespace_)); + rmw_node_free(node); return RMW_RET_OK; } From 468c493403868be1f50ae576cb908195f42b2fd2 Mon Sep 17 00:00:00 2001 From: Michel Hidalgo Date: Mon, 13 Jul 2020 18:59:11 -0300 Subject: [PATCH 4/7] Drop unnecessary assertions. Signed-off-by: Michel Hidalgo --- rmw_fastrtps_cpp/src/rmw_node.cpp | 1 - rmw_fastrtps_dynamic_cpp/src/rmw_node.cpp | 1 - rmw_fastrtps_shared_cpp/src/rmw_node.cpp | 6 +----- 3 files changed, 1 insertion(+), 7 deletions(-) diff --git a/rmw_fastrtps_cpp/src/rmw_node.cpp b/rmw_fastrtps_cpp/src/rmw_node.cpp index 2652965e5..1fcf18687 100644 --- a/rmw_fastrtps_cpp/src/rmw_node.cpp +++ b/rmw_fastrtps_cpp/src/rmw_node.cpp @@ -83,7 +83,6 @@ rmw_destroy_node(rmw_node_t * node) node->implementation_identifier, eprosima_fastrtps_identifier, return RMW_RET_INCORRECT_RMW_IMPLEMENTATION); - assert(node->context != nullptr); rmw_context_t * context = node->context; rmw_ret_t ret = rmw_fastrtps_shared_cpp::__rmw_destroy_node( diff --git a/rmw_fastrtps_dynamic_cpp/src/rmw_node.cpp b/rmw_fastrtps_dynamic_cpp/src/rmw_node.cpp index 94f914f5d..6c492acae 100644 --- a/rmw_fastrtps_dynamic_cpp/src/rmw_node.cpp +++ b/rmw_fastrtps_dynamic_cpp/src/rmw_node.cpp @@ -83,7 +83,6 @@ rmw_destroy_node(rmw_node_t * node) node->implementation_identifier, eprosima_fastrtps_identifier, return RMW_RET_INCORRECT_RMW_IMPLEMENTATION); - assert(node->context != nullptr); rmw_context_t * context = node->context; rmw_ret_t ret = rmw_fastrtps_shared_cpp::__rmw_destroy_node( eprosima_fastrtps_identifier, node); diff --git a/rmw_fastrtps_shared_cpp/src/rmw_node.cpp b/rmw_fastrtps_shared_cpp/src/rmw_node.cpp index a21c8314d..7b5e8aa3c 100644 --- a/rmw_fastrtps_shared_cpp/src/rmw_node.cpp +++ b/rmw_fastrtps_shared_cpp/src/rmw_node.cpp @@ -134,11 +134,7 @@ __rmw_destroy_node( { assert(node != nullptr); assert(node->implementation_identifier == identifier); - static_cast(identifier); - assert(node->context != nullptr); - assert(node->context->impl != nullptr); - assert(node->context->impl->common != nullptr); auto common_context = static_cast(node->context->impl->common); rmw_dds_common::GraphCache & graph_cache = common_context->graph_cache; { @@ -146,7 +142,7 @@ __rmw_destroy_node( rmw_dds_common::msg::ParticipantEntitiesInfo participant_msg = graph_cache.remove_node(common_context->gid, node->name, node->namespace_); rmw_ret_t ret = __rmw_publish( - node->implementation_identifier, + identifier, common_context->pub, static_cast(&participant_msg), nullptr); From dc6d83888396c75d82123a395a857bb3431276d4 Mon Sep 17 00:00:00 2001 From: Michel Hidalgo Date: Tue, 14 Jul 2020 10:27:13 -0300 Subject: [PATCH 5/7] Drop a few more unnecessary assertions. Signed-off-by: Michel Hidalgo --- rmw_fastrtps_shared_cpp/src/rmw_node.cpp | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/rmw_fastrtps_shared_cpp/src/rmw_node.cpp b/rmw_fastrtps_shared_cpp/src/rmw_node.cpp index 7b5e8aa3c..9d6e4c784 100644 --- a/rmw_fastrtps_shared_cpp/src/rmw_node.cpp +++ b/rmw_fastrtps_shared_cpp/src/rmw_node.cpp @@ -46,10 +46,7 @@ __rmw_create_node( const char * name, const char * namespace_) { - assert(nullptr != context); - assert(nullptr != context->impl); assert(identifier == context->implementation_identifier); - static_cast(identifier); int validation_result = RMW_NODE_NAME_VALID; rmw_ret_t ret = rmw_validate_node_name(name, &validation_result, nullptr); @@ -84,7 +81,7 @@ __rmw_create_node( rmw_free(const_cast(node_handle->namespace_)); rmw_node_free(node_handle); }); - node_handle->implementation_identifier = context->implementation_identifier; + node_handle->implementation_identifier = identifier; node_handle->data = nullptr; node_handle->name = @@ -132,7 +129,6 @@ __rmw_destroy_node( const char * identifier, rmw_node_t * node) { - assert(node != nullptr); assert(node->implementation_identifier == identifier); auto common_context = static_cast(node->context->impl->common); From 465fb7a4ff57747e46d9884e3eaa7699c39d191a Mon Sep 17 00:00:00 2001 From: Michel Hidalgo Date: Tue, 14 Jul 2020 13:14:25 -0300 Subject: [PATCH 6/7] Fix clang warnings. Signed-off-by: Michel Hidalgo --- rmw_fastrtps_shared_cpp/src/rmw_node.cpp | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-) diff --git a/rmw_fastrtps_shared_cpp/src/rmw_node.cpp b/rmw_fastrtps_shared_cpp/src/rmw_node.cpp index 9d6e4c784..592871b09 100644 --- a/rmw_fastrtps_shared_cpp/src/rmw_node.cpp +++ b/rmw_fastrtps_shared_cpp/src/rmw_node.cpp @@ -50,19 +50,21 @@ __rmw_create_node( int validation_result = RMW_NODE_NAME_VALID; rmw_ret_t ret = rmw_validate_node_name(name, &validation_result, nullptr); - if (RMW_RET_OK != ret || RMW_NODE_NAME_VALID != validation_result) { - const char * reason = RMW_RET_OK == ret ? - rmw_node_name_validation_result_string(validation_result) : - rmw_get_error_string().str; + if (RMW_RET_OK != ret) { + return nullptr; + } + if (RMW_NODE_NAME_VALID != validation_result) { + const char * reason = rmw_node_name_validation_result_string(validation_result); RMW_SET_ERROR_MSG_WITH_FORMAT_STRING("invalid node name: %s", reason); return nullptr; } validation_result = RMW_NAMESPACE_VALID; ret = rmw_validate_namespace(namespace_, &validation_result, nullptr); - if (RMW_RET_OK != ret || RMW_NAMESPACE_VALID != validation_result) { - const char * reason = RMW_RET_OK == ret ? - rmw_node_name_validation_result_string(validation_result) : - rmw_get_error_string().str; + if (RMW_RET_OK != ret) { + return nullptr; + } + if (RMW_NAMESPACE_VALID != validation_result) { + const char * reason = rmw_node_name_validation_result_string(validation_result); RMW_SET_ERROR_MSG_WITH_FORMAT_STRING("invalid node namespace: %s", reason); return nullptr; } From 8ee8b24539fc0d8792b3a5fe1649d535292462db Mon Sep 17 00:00:00 2001 From: Michel Hidalgo Date: Tue, 14 Jul 2020 18:12:23 -0300 Subject: [PATCH 7/7] Fix one last clang warning. Signed-off-by: Michel Hidalgo --- rmw_fastrtps_shared_cpp/src/rmw_node.cpp | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/rmw_fastrtps_shared_cpp/src/rmw_node.cpp b/rmw_fastrtps_shared_cpp/src/rmw_node.cpp index 592871b09..7a3a4f915 100644 --- a/rmw_fastrtps_shared_cpp/src/rmw_node.cpp +++ b/rmw_fastrtps_shared_cpp/src/rmw_node.cpp @@ -71,14 +71,13 @@ __rmw_create_node( auto common_context = static_cast(context->impl->common); rmw_dds_common::GraphCache & graph_cache = common_context->graph_cache; - rcutils_allocator_t allocator = context->options.allocator; rmw_node_t * node_handle = rmw_node_allocate(); if (nullptr == node_handle) { RMW_SET_ERROR_MSG("failed to allocate node"); return nullptr; } auto cleanup_node = rcpputils::make_scope_exit( - [node_handle, allocator]() { + [node_handle]() { rmw_free(const_cast(node_handle->name)); rmw_free(const_cast(node_handle->namespace_)); rmw_node_free(node_handle);