diff --git a/rcl/include/rcl/client.h b/rcl/include/rcl/client.h index 16a8059..644e578 100644 --- a/rcl/include/rcl/client.h +++ b/rcl/include/rcl/client.h @@ -177,6 +177,7 @@ rcl_client_init( * \param[in] node handle to the node used to create the client * \return `RCL_RET_OK` if client was finalized successfully, or * \return `RCL_RET_INVALID_ARGUMENT` if any arguments are invalid, or + * \return `RCL_RET_NODE_INVALID` if the node is invalid, or * \return `RCL_RET_ERROR` if an unspecified error occurs. */ RCL_PUBLIC diff --git a/rcl/include/rcl/node.h b/rcl/include/rcl/node.h index 0a14252..c1c079e 100644 --- a/rcl/include/rcl/node.h +++ b/rcl/include/rcl/node.h @@ -203,7 +203,11 @@ rcl_node_get_default_options(void); /// Return `true` if the node is valid, else `false`. /** - * Also return `false` if the node pointer is `NULL`. + * Also return `false` if the node pointer is `NULL` or the allocator is invalid. + * + * The allocator needs to either be a valid allocator or `NULL`, in which case + * the default allocator will be used. + * The allocator is used when allocation is needed for an error message. * * A node is invalid if: * - the implementation is `NULL` (rcl_node_init not called or failed) @@ -215,7 +219,7 @@ rcl_node_get_default_options(void); * Consider: * * ```c - * assert(rcl_node_is_valid(node)); // <-- thread 1 + * assert(rcl_node_is_valid(node, NULL)); // <-- thread 1 * rcl_shutdown(); // <-- thread 2 * // use node as if valid // <-- thread 1 * ``` @@ -234,11 +238,12 @@ rcl_node_get_default_options(void); * [1] if `atomic_is_lock_free()` returns true for `atomic_uint_least64_t` * * \param[in] node rcl_node_t to be validated - * \return `true` if the node is valid, otherwise `false`. + * \param[in] allocator a valid allocator or `NULL` + * \return `true` if the node and allocator are valid, otherwise `false`. */ RCL_PUBLIC bool -rcl_node_is_valid(const rcl_node_t * node); +rcl_node_is_valid(const rcl_node_t * node, const rcl_allocator_t * allocator); /// Return the name of the node. /** diff --git a/rcl/include/rcl/publisher.h b/rcl/include/rcl/publisher.h index d3d1a04..a6ebafe 100644 --- a/rcl/include/rcl/publisher.h +++ b/rcl/include/rcl/publisher.h @@ -171,6 +171,7 @@ rcl_publisher_init( * \param[in] node handle to the node used to create the publisher * \return `RCL_RET_OK` if publisher was finalized successfully, or * \return `RCL_RET_INVALID_ARGUMENT` if any arguments are invalid, or + * \return `RCL_RET_NODE_INVALID` if the node is invalid, or * \return `RCL_RET_ERROR` if an unspecified error occurs. */ RCL_PUBLIC diff --git a/rcl/include/rcl/service.h b/rcl/include/rcl/service.h index fd9383a..69da1b9 100644 --- a/rcl/include/rcl/service.h +++ b/rcl/include/rcl/service.h @@ -138,6 +138,7 @@ rcl_get_zero_initialized_service(void); * \param[in] options service options, including quality of service settings * \return `RCL_RET_OK` if service was initialized successfully, or * \return `RCL_RET_INVALID_ARGUMENT` if any arguments are invalid, or + * \return `RCL_RET_NODE_INVALID` if the node is invalid, or * \return `RCL_RET_BAD_ALLOC` if allocating memory failed, or * \return `RCL_RET_SERVICE_NAME_INVALID` if the given service name is invalid, or * \return `RCL_RET_ERROR` if an unspecified error occurs. @@ -174,6 +175,7 @@ rcl_service_init( * \param[in] node handle to the node used to create the service * \return `RCL_RET_OK` if service was deinitialized successfully, or * \return `RCL_RET_INVALID_ARGUMENT` if any arguments are invalid, or + * \return `RCL_RET_NODE_INVALID` if the node is invalid, or * \return `RCL_RET_ERROR` if an unspecified error occurs. */ RCL_PUBLIC diff --git a/rcl/include/rcl/subscription.h b/rcl/include/rcl/subscription.h index 76390e4..d4af20e 100644 --- a/rcl/include/rcl/subscription.h +++ b/rcl/include/rcl/subscription.h @@ -140,6 +140,7 @@ rcl_get_zero_initialized_subscription(void); * \param[in] options subscription options, including quality of service settings * \return `RCL_RET_OK` if subscription was initialized successfully, or * \return `RCL_RET_INVALID_ARGUMENT` if any arguments are invalid, or + * \return `RCL_RET_NODE_INVALID` if the node is invalid, or * \return `RCL_RET_BAD_ALLOC` if allocating memory failed, or * \return `RCL_RET_TOPIC_NAME_INVALID` if the given topic name is invalid, or * \return `RCL_RET_ERROR` if an unspecified error occurs. @@ -176,6 +177,7 @@ rcl_subscription_init( * \param[in] node handle to the node used to create the subscription * \return `RCL_RET_OK` if subscription was deinitialized successfully, or * \return `RCL_RET_INVALID_ARGUMENT` if any arguments are invalid, or + * \return `RCL_RET_NODE_INVALID` if the node is invalid, or * \return `RCL_RET_ERROR` if an unspecified error occurs. */ RCL_PUBLIC diff --git a/rcl/src/rcl/client.c b/rcl/src/rcl/client.c index 27153b6..1ffd254 100644 --- a/rcl/src/rcl/client.c +++ b/rcl/src/rcl/client.c @@ -62,8 +62,7 @@ rcl_client_init( RCL_CHECK_ALLOCATOR_WITH_MSG(allocator, "invalid allocator", return RCL_RET_INVALID_ARGUMENT); RCL_CHECK_ARGUMENT_FOR_NULL(client, RCL_RET_INVALID_ARGUMENT, *allocator); RCL_CHECK_ARGUMENT_FOR_NULL(node, RCL_RET_INVALID_ARGUMENT, *allocator); - if (!node->impl) { - RCL_SET_ERROR_MSG("invalid node", *allocator); + if (!rcl_node_is_valid(node, allocator)) { return RCL_RET_NODE_INVALID; } RCL_CHECK_ARGUMENT_FOR_NULL(type_support, RCL_RET_INVALID_ARGUMENT, *allocator); @@ -167,6 +166,9 @@ rcl_client_fini(rcl_client_t * client, rcl_node_t * node) rcl_ret_t result = RCL_RET_OK; RCL_CHECK_ARGUMENT_FOR_NULL(client, RCL_RET_INVALID_ARGUMENT, rcl_get_default_allocator()); RCL_CHECK_ARGUMENT_FOR_NULL(node, RCL_RET_INVALID_ARGUMENT, rcl_get_default_allocator()); + if (!rcl_node_is_valid(node, NULL)) { + return RCL_RET_NODE_INVALID; + } if (client->impl) { rcl_allocator_t allocator = client->impl->options.allocator; rmw_node_t * rmw_node = rcl_node_get_rmw_handle(node); diff --git a/rcl/src/rcl/graph.c b/rcl/src/rcl/graph.c index fd226a9..ea975b4 100644 --- a/rcl/src/rcl/graph.c +++ b/rcl/src/rcl/graph.c @@ -37,8 +37,8 @@ rcl_get_topic_names_and_types( rcl_names_and_types_t * topic_names_and_types) { RCL_CHECK_ARGUMENT_FOR_NULL(allocator, RCL_RET_INVALID_ARGUMENT, rcl_get_default_allocator()) - RCL_CHECK_ARGUMENT_FOR_NULL(node, RCL_RET_INVALID_ARGUMENT, *allocator) - if (!rcl_node_is_valid(node)) { + RCL_CHECK_ARGUMENT_FOR_NULL(node, RCL_RET_INVALID_ARGUMENT, *allocator); + if (!rcl_node_is_valid(node, allocator)) { return RCL_RET_NODE_INVALID; } RCL_CHECK_ARGUMENT_FOR_NULL(topic_names_and_types, RCL_RET_INVALID_ARGUMENT, *allocator) @@ -64,7 +64,7 @@ rcl_get_service_names_and_types( rcl_names_and_types_t * service_names_and_types) { RCL_CHECK_ARGUMENT_FOR_NULL(node, RCL_RET_INVALID_ARGUMENT, *allocator); - if (!rcl_node_is_valid(node)) { + if (!rcl_node_is_valid(node, allocator)) { return RCL_RET_NODE_INVALID; } RCL_CHECK_ARGUMENT_FOR_NULL(service_names_and_types, RCL_RET_INVALID_ARGUMENT, *allocator); @@ -98,7 +98,7 @@ rcl_get_node_names( rcutils_string_array_t * node_names) { RCL_CHECK_ARGUMENT_FOR_NULL(node, RCL_RET_INVALID_ARGUMENT, allocator); - if (!rcl_node_is_valid(node)) { + if (!rcl_node_is_valid(node, &allocator)) { return RCL_RET_NODE_INVALID; } RCL_CHECK_ARGUMENT_FOR_NULL(node_names, RCL_RET_INVALID_ARGUMENT, allocator); @@ -123,7 +123,7 @@ rcl_count_publishers( size_t * count) { RCL_CHECK_ARGUMENT_FOR_NULL(node, RCL_RET_INVALID_ARGUMENT, rcl_get_default_allocator()); - if (!rcl_node_is_valid(node)) { + if (!rcl_node_is_valid(node, NULL)) { return RCL_RET_NODE_INVALID; } const rcl_node_options_t * node_options = rcl_node_get_options(node); @@ -143,7 +143,7 @@ rcl_count_subscribers( size_t * count) { RCL_CHECK_ARGUMENT_FOR_NULL(node, RCL_RET_INVALID_ARGUMENT, rcl_get_default_allocator()); - if (!rcl_node_is_valid(node)) { + if (!rcl_node_is_valid(node, NULL)) { return RCL_RET_NODE_INVALID; } const rcl_node_options_t * node_options = rcl_node_get_options(node); @@ -163,7 +163,7 @@ rcl_service_server_is_available( bool * is_available) { RCL_CHECK_ARGUMENT_FOR_NULL(node, RCL_RET_INVALID_ARGUMENT, rcl_get_default_allocator()); - if (!rcl_node_is_valid(node)) { + if (!rcl_node_is_valid(node, NULL)) { return RCL_RET_NODE_INVALID; } const rcl_node_options_t * node_options = rcl_node_get_options(node); diff --git a/rcl/src/rcl/node.c b/rcl/src/rcl/node.c index 72ae5db..952afc9 100644 --- a/rcl/src/rcl/node.c +++ b/rcl/src/rcl/node.c @@ -280,10 +280,9 @@ rcl_node_init( node->impl->rcl_instance_id = rcl_get_instance_id(); // graph guard condition rmw_graph_guard_condition = rmw_node_get_graph_guard_condition(node->impl->rmw_node_handle); - if (!rmw_graph_guard_condition) { - RCL_SET_ERROR_MSG(rmw_get_error_string_safe(), *allocator); - goto fail; - } + RCL_CHECK_FOR_NULL_WITH_MSG( + rmw_graph_guard_condition, rmw_get_error_string_safe(), goto fail, *allocator); + node->impl->graph_guard_condition = (rcl_guard_condition_t *)allocator->allocate( sizeof(rcl_guard_condition_t), allocator->state); RCL_CHECK_FOR_NULL_WITH_MSG( @@ -363,14 +362,16 @@ rcl_node_fini(rcl_node_t * node) } bool -rcl_node_is_valid(const rcl_node_t * node) +rcl_node_is_valid(const rcl_node_t * node, const rcl_allocator_t * allocator) { - RCL_CHECK_ARGUMENT_FOR_NULL(node, false, rcl_get_default_allocator()); + rcl_allocator_t alloc = allocator ? *allocator : rcl_get_default_allocator(); + RCL_CHECK_ALLOCATOR_WITH_MSG(&alloc, "invalid allocator", return false); + RCL_CHECK_ARGUMENT_FOR_NULL(node, false, alloc); RCL_CHECK_FOR_NULL_WITH_MSG( - node->impl, "rcl node implementation is invalid", return false, rcl_get_default_allocator()); + node->impl, "rcl node implementation is invalid", return false, alloc); if (node->impl->rcl_instance_id != rcl_get_instance_id()) { RCL_SET_ERROR_MSG( - "rcl node is invalid, rcl instance id does not match", rcl_get_default_allocator()); + "rcl node is invalid, rcl instance id does not match", alloc); return false; } return true; @@ -391,7 +392,7 @@ rcl_node_get_default_options() const char * rcl_node_get_name(const rcl_node_t * node) { - if (!rcl_node_is_valid(node)) { + if (!rcl_node_is_valid(node, NULL)) { return NULL; } return node->impl->rmw_node_handle->name; @@ -400,7 +401,7 @@ rcl_node_get_name(const rcl_node_t * node) const char * rcl_node_get_namespace(const rcl_node_t * node) { - if (!rcl_node_is_valid(node)) { + if (!rcl_node_is_valid(node, NULL)) { return NULL; } return node->impl->rmw_node_handle->namespace_; @@ -409,7 +410,7 @@ rcl_node_get_namespace(const rcl_node_t * node) const rcl_node_options_t * rcl_node_get_options(const rcl_node_t * node) { - if (!rcl_node_is_valid(node)) { + if (!rcl_node_is_valid(node, NULL)) { return NULL; } return &node->impl->options; @@ -431,7 +432,7 @@ rcl_node_get_domain_id(const rcl_node_t * node, size_t * domain_id) rmw_node_t * rcl_node_get_rmw_handle(const rcl_node_t * node) { - if (!rcl_node_is_valid(node)) { + if (!rcl_node_is_valid(node, NULL)) { return NULL; } return node->impl->rmw_node_handle; @@ -449,7 +450,7 @@ rcl_node_get_rcl_instance_id(const rcl_node_t * node) const struct rcl_guard_condition_t * rcl_node_get_graph_guard_condition(const rcl_node_t * node) { - if (!rcl_node_is_valid(node)) { + if (!rcl_node_is_valid(node, NULL)) { return NULL; } return node->impl->graph_guard_condition; diff --git a/rcl/src/rcl/publisher.c b/rcl/src/rcl/publisher.c index ff7e2d9..d30ad2e 100644 --- a/rcl/src/rcl/publisher.c +++ b/rcl/src/rcl/publisher.c @@ -61,12 +61,11 @@ rcl_publisher_init( RCL_CHECK_ARGUMENT_FOR_NULL(publisher, RCL_RET_INVALID_ARGUMENT, *allocator); if (publisher->impl) { RCL_SET_ERROR_MSG( - "publisher already initialized, or memory was unintialized", rcl_get_default_allocator()); + "publisher already initialized, or memory was unintialized", *allocator); return RCL_RET_ALREADY_INIT; } RCL_CHECK_ARGUMENT_FOR_NULL(node, RCL_RET_INVALID_ARGUMENT, *allocator); - if (!node->impl) { - RCL_SET_ERROR_MSG("invalid node", *allocator); + if (!rcl_node_is_valid(node, allocator)) { return RCL_RET_NODE_INVALID; } RCL_CHECK_ARGUMENT_FOR_NULL(type_support, RCL_RET_INVALID_ARGUMENT, *allocator); @@ -145,10 +144,8 @@ rcl_publisher_init( expanded_topic_name, &(options->qos)); allocator->deallocate(expanded_topic_name, allocator->state); - if (!publisher->impl->rmw_handle) { - RCL_SET_ERROR_MSG(rmw_get_error_string_safe(), *allocator); - goto fail; - } + RCL_CHECK_FOR_NULL_WITH_MSG(publisher->impl->rmw_handle, + rmw_get_error_string_safe(), goto fail, *allocator); // options publisher->impl->options = *options; return RCL_RET_OK; @@ -165,6 +162,9 @@ rcl_publisher_fini(rcl_publisher_t * publisher, rcl_node_t * node) rcl_ret_t result = RCL_RET_OK; RCL_CHECK_ARGUMENT_FOR_NULL(publisher, RCL_RET_INVALID_ARGUMENT, rcl_get_default_allocator()); RCL_CHECK_ARGUMENT_FOR_NULL(node, RCL_RET_INVALID_ARGUMENT, rcl_get_default_allocator()); + if (!rcl_node_is_valid(node, NULL)) { + return RCL_RET_NODE_INVALID; + } if (publisher->impl) { rcl_allocator_t allocator = publisher->impl->options.allocator; rmw_node_t * rmw_node = rcl_node_get_rmw_handle(node); diff --git a/rcl/src/rcl/service.c b/rcl/src/rcl/service.c index fc89d00..9b58529 100644 --- a/rcl/src/rcl/service.c +++ b/rcl/src/rcl/service.c @@ -59,8 +59,7 @@ rcl_service_init( RCL_CHECK_ARGUMENT_FOR_NULL(service, RCL_RET_INVALID_ARGUMENT, *allocator); RCL_CHECK_ARGUMENT_FOR_NULL(node, RCL_RET_INVALID_ARGUMENT, *allocator); - if (!node->impl) { - RCL_SET_ERROR_MSG("invalid node", *allocator); + if (!rcl_node_is_valid(node, allocator)) { return RCL_RET_NODE_INVALID; } RCL_CHECK_ARGUMENT_FOR_NULL(type_support, RCL_RET_INVALID_ARGUMENT, *allocator); @@ -170,6 +169,9 @@ rcl_service_fini(rcl_service_t * service, rcl_node_t * node) rcl_ret_t result = RCL_RET_OK; RCL_CHECK_ARGUMENT_FOR_NULL(service, RCL_RET_INVALID_ARGUMENT, rcl_get_default_allocator()); RCL_CHECK_ARGUMENT_FOR_NULL(node, RCL_RET_INVALID_ARGUMENT, rcl_get_default_allocator()); + if (!rcl_node_is_valid(node, NULL)) { + return RCL_RET_NODE_INVALID; + } if (service->impl) { rcl_allocator_t allocator = service->impl->options.allocator; rmw_node_t * rmw_node = rcl_node_get_rmw_handle(node); diff --git a/rcl/src/rcl/subscription.c b/rcl/src/rcl/subscription.c index e7d22de..eba950b 100644 --- a/rcl/src/rcl/subscription.c +++ b/rcl/src/rcl/subscription.c @@ -55,9 +55,12 @@ rcl_subscription_init( RCL_CHECK_ARGUMENT_FOR_NULL(options, RCL_RET_INVALID_ARGUMENT, rcl_get_default_allocator()); const rcl_allocator_t * allocator = &options->allocator; RCL_CHECK_ALLOCATOR_WITH_MSG(allocator, "invalid allocator", return RCL_RET_INVALID_ARGUMENT); - RCL_CHECK_ARGUMENT_FOR_NULL(subscription, RCL_RET_INVALID_ARGUMENT, *allocator); RCL_CHECK_ARGUMENT_FOR_NULL(node, RCL_RET_INVALID_ARGUMENT, *allocator); + if (!rcl_node_is_valid(node, allocator)) { + return RCL_RET_NODE_INVALID; + } + RCL_CHECK_ARGUMENT_FOR_NULL(subscription, RCL_RET_INVALID_ARGUMENT, *allocator); RCL_CHECK_ARGUMENT_FOR_NULL(type_support, RCL_RET_INVALID_ARGUMENT, *allocator); RCL_CHECK_ARGUMENT_FOR_NULL(topic_name, RCL_RET_INVALID_ARGUMENT, *allocator); if (subscription->impl) { @@ -159,6 +162,9 @@ rcl_subscription_fini(rcl_subscription_t * subscription, rcl_node_t * node) rcl_ret_t result = RCL_RET_OK; RCL_CHECK_ARGUMENT_FOR_NULL(subscription, RCL_RET_INVALID_ARGUMENT, rcl_get_default_allocator()); RCL_CHECK_ARGUMENT_FOR_NULL(node, RCL_RET_INVALID_ARGUMENT, rcl_get_default_allocator()); + if (!rcl_node_is_valid(node, NULL)) { + return RCL_RET_NODE_INVALID; + } if (subscription->impl) { rcl_allocator_t allocator = subscription->impl->options.allocator; rmw_node_t * rmw_node = rcl_node_get_rmw_handle(node); diff --git a/rcl/test/rcl/test_node.cpp b/rcl/test/rcl/test_node.cpp index a5f9162..ad5c3b6 100644 --- a/rcl/test/rcl/test_node.cpp +++ b/rcl/test/rcl/test_node.cpp @@ -118,16 +118,16 @@ TEST_F(CLASSNAME(TestNodeFixture, RMW_IMPLEMENTATION), test_rcl_node_accessors) }); // Test rcl_node_is_valid(). bool is_valid; - is_valid = rcl_node_is_valid(nullptr); + is_valid = rcl_node_is_valid(nullptr, nullptr); EXPECT_FALSE(is_valid); rcl_reset_error(); - is_valid = rcl_node_is_valid(&zero_node); + is_valid = rcl_node_is_valid(&zero_node, nullptr); EXPECT_FALSE(is_valid); rcl_reset_error(); - is_valid = rcl_node_is_valid(&invalid_node); + is_valid = rcl_node_is_valid(&invalid_node, nullptr); EXPECT_FALSE(is_valid); rcl_reset_error(); - is_valid = rcl_node_is_valid(&node); + is_valid = rcl_node_is_valid(&node, nullptr); EXPECT_TRUE(is_valid); rcl_reset_error(); // Test rcl_node_get_name().