Skip to content

Commit

Permalink
Ensure compliant node construction/destruction API. (#408)
Browse files Browse the repository at this point in the history 
Signed-off-by: Michel Hidalgo <[email protected]>
  • Loading branch information
@hidmic @ahcorde
hidmic authored and ahcorde committed Oct 15, 2020
1 parent 5629cab commit 8394f52
Show file tree
Hide file tree
Showing 3 changed files with 80 additions and 48 deletions.
18 changes: 17 additions & 1 deletion rmw_fastrtps_cpp/src/rmw_node.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -44,13 +45,21 @@ rmw_create_node(
{
(void)domain_id;
(void)localhost_only;
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;
Expand All @@ -72,6 +81,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);

rmw_context_t * context = node->context;
rmw_ret_t ret = rmw_fastrtps_shared_cpp::__rmw_destroy_node(
eprosima_fastrtps_identifier, node);
Expand Down
19 changes: 17 additions & 2 deletions rmw_fastrtps_dynamic_cpp/src/rmw_node.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -44,13 +45,21 @@ rmw_create_node(
{
(void)domain_id;
(void)localhost_only;
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;
Expand All @@ -72,6 +81,12 @@ 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);
rmw_context_t * context = node->context;
rmw_ret_t ret = rmw_fastrtps_shared_cpp::__rmw_destroy_node(
eprosima_fastrtps_identifier, node);
Expand Down
91 changes: 46 additions & 45 deletions rmw_fastrtps_shared_cpp/src/rmw_node.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,10 @@
#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"

Expand All @@ -42,44 +46,61 @@ __rmw_create_node(
const char * name,
const char * namespace_)
{
if (!name) {
RMW_SET_ERROR_MSG("name is null");
assert(identifier == context->implementation_identifier);

int validation_result = RMW_NODE_NAME_VALID;
rmw_ret_t ret = rmw_validate_node_name(name, &validation_result, nullptr);
if (RMW_RET_OK != ret) {
return nullptr;
}

if (!namespace_) {
RMW_SET_ERROR_MSG("namespace_ is null");
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) {
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;
}

rmw_node_t * node_handle = nullptr;
auto common_context = static_cast<rmw_dds_common::Context *>(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;
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]() {
rmw_free(const_cast<char *>(node_handle->name));
rmw_free(const_cast<char *>(node_handle->namespace_));
rmw_node_free(node_handle);
});
node_handle->implementation_identifier = identifier;
node_handle->data = nullptr;

node_handle->name =
static_cast<const char *>(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;
if (nullptr == node_handle->name) {
RMW_SET_ERROR_MSG("failed to copy node name");
return nullptr;
}
memcpy(const_cast<char *>(node_handle->name), name, strlen(name) + 1);

node_handle->namespace_ =
static_cast<const char *>(rmw_allocate(sizeof(char) * strlen(namespace_) + 1));
if (!node_handle->namespace_) {
RMW_SET_ERROR_MSG("failed to allocate memory");
goto fail;
if (nullptr == node_handle->namespace_) {
RMW_SET_ERROR_MSG("failed to copy node namespace");
return nullptr;
}
memcpy(const_cast<char *>(node_handle->namespace_), namespace_, strlen(namespace_) + 1);

node_handle->context = context;

{
Expand All @@ -92,63 +113,43 @@ __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_handle->implementation_identifier,
common_context->pub,
static_cast<void *>(&participant_msg),
nullptr))
{
goto fail;
return nullptr;
}
}
cleanup_node.cancel();
return node_handle;
fail:
if (node_handle) {
rmw_free(const_cast<char *>(node_handle->namespace_));
node_handle->namespace_ = nullptr;
rmw_free(const_cast<char *>(node_handle->name));
node_handle->name = nullptr;
}
rmw_node_free(node_handle);
return nullptr;
}

rmw_ret_t
__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->implementation_identifier == identifier);

auto common_context = static_cast<rmw_dds_common::Context *>(node->context->impl->common);
rmw_dds_common::GraphCache & graph_cache = common_context->graph_cache;
{
std::lock_guard<std::mutex> 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(
rmw_ret_t ret = __rmw_publish(
identifier,
common_context->pub,
static_cast<void *>(&participant_msg),
nullptr);
if (RMW_RET_OK != result_ret) {
return result_ret;
if (RMW_RET_OK != ret) {
return ret;
}
}
rmw_free(const_cast<char *>(node->name));
node->name = nullptr;
rmw_free(const_cast<char *>(node->namespace_));
node->namespace_ = nullptr;
rmw_node_free(node);

return RMW_RET_OK;
}

Expand Down

0 comments on commit 8394f52

Please sign in to comment.