From e47f644faabc6d99ec6e563105eb51eef6188af1 Mon Sep 17 00:00:00 2001 From: dhood Date: Mon, 26 Feb 2018 14:36:25 -0800 Subject: [PATCH] Store logger name associated with node (#212) * Give node logger name * Expose getter * Workaround to fix non-const warning * Add test for node logger name * Move to function * More test coverage * comment fixup * Restructure logger name logic * Document input assumptions * Don't hard-code logger name separator * Remove const workaround --- rcl/include/rcl/node.h | 27 +++++++++ rcl/src/rcl/node.c | 64 +++++++++++++++++++++ rcl/test/rcl/test_node.cpp | 114 +++++++++++++++++++++++++++++++++++++ 3 files changed, 205 insertions(+) diff --git a/rcl/include/rcl/node.h b/rcl/include/rcl/node.h index deddc92..49ebb2f 100644 --- a/rcl/include/rcl/node.h +++ b/rcl/include/rcl/node.h @@ -452,6 +452,33 @@ RCL_WARN_UNUSED const struct rcl_guard_condition_t * rcl_node_get_graph_guard_condition(const rcl_node_t * node); +/// Return the logger name of the node. +/** + * This function returns the node's internal logger name string. + * This function can fail, and therefore return `NULL`, if: + * - node is `NULL` + * - node has not been initialized (the implementation is invalid) + * + * The returned string is only valid as long as the given rcl_node_t is valid. + * The value of the string may change if the value in the rcl_node_t changes, + * and therefore copying the string is recommended if this is a concern. + * + *
+ * Attribute | Adherence + * ------------------ | ------------- + * Allocates Memory | No + * Thread-Safe | No + * Uses Atomics | No + * Lock-Free | Yes + * + * \param[in] node pointer to the node + * \return logger_name string if successful, otherwise `NULL` + */ +RCL_PUBLIC +RCL_WARN_UNUSED +const char * +rcl_node_get_logger_name(const rcl_node_t * node); + #if __cplusplus } #endif diff --git a/rcl/src/rcl/node.c b/rcl/src/rcl/node.c index 981973f..5af2724 100644 --- a/rcl/src/rcl/node.c +++ b/rcl/src/rcl/node.c @@ -27,11 +27,14 @@ extern "C" #include "rcl/error_handling.h" #include "rcl/rcl.h" #include "rcutils/filesystem.h" +#include "rcutils/find.h" #include "rcutils/format_string.h" #include "rcutils/get_env.h" #include "rcutils/logging_macros.h" #include "rcutils/macros.h" +#include "rcutils/repl_str.h" #include "rcutils/snprintf.h" +#include "rcutils/strdup.h" #include "rmw/error_handling.h" #include "rmw/node_security_options.h" #include "rmw/rmw.h" @@ -52,9 +55,51 @@ typedef struct rcl_node_impl_t rmw_node_t * rmw_node_handle; uint64_t rcl_instance_id; rcl_guard_condition_t * graph_guard_condition; + const char * logger_name; } rcl_node_impl_t; +/// Return the logger name associated with a node given the validated node name and namespace. +/** + * E.g. for a node named "c" in namespace "/a/b", the logger name will be + * "a.b.c", assuming logger name separator of ".". + * + * \param[in] node_name validated node name (a single token) + * \param[in] node_namespace validated, absolute namespace (starting with "/") + * \param[in] allocator the allocator to use for allocation + * \returns duplicated string or null if there is an error + */ +const char * rcl_create_node_logger_name( + const char * node_name, + const char * node_namespace, + const rcl_allocator_t * allocator) +{ + // If the namespace is the root namespace ("/"), the logger name is just the node name. + if (strlen(node_namespace) == 1) { + return rcutils_strdup(node_name, *allocator); + } + + // Convert the forward slashes in the namespace to the separator used for logger names. + // The input namespace has already been expanded and therefore will always be absolute, + // i.e. it will start with a forward slash, which we want to ignore. + const char * ns_with_separators = rcutils_repl_str( + node_namespace + 1, // Ignore the leading forward slash. + "/", RCUTILS_LOGGING_SEPARATOR_STRING, + allocator); + if (NULL == ns_with_separators) { + return NULL; + } + + // Join the namespace and node name to create the logger name. + char * node_logger_name = rcutils_format_string( + *allocator, "%s%s%s", ns_with_separators, RCUTILS_LOGGING_SEPARATOR_STRING, node_name); + if (NULL == node_logger_name) { + allocator->deallocate((char *)ns_with_separators, allocator->state); + return NULL; + } + return node_logger_name; +} + const char * rcl_get_secure_root(const char * node_name) { const char * ros_secure_root_env = NULL; @@ -187,6 +232,12 @@ rcl_node_init( // Initialize node impl. // node options (assume it is trivially copyable) node->impl->options = *options; + + // node logger name + node->impl->logger_name = rcl_create_node_logger_name(name, local_namespace_, allocator); + RCL_CHECK_FOR_NULL_WITH_MSG( + node->impl->logger_name, "creating logger name failed", goto fail, *allocator); + // node rmw_node_handle if (node->impl->options.domain_id == RCL_NODE_OPTIONS_DEFAULT_DOMAIN_ID) { // Find the domain ID set by the environment. @@ -300,6 +351,9 @@ rcl_node_init( return RCL_RET_OK; fail: if (node->impl) { + if (node->impl->logger_name) { + allocator->deallocate((char *)node->impl->logger_name, allocator->state); + } if (node->impl->rmw_node_handle) { ret = rmw_destroy_node(node->impl->rmw_node_handle); if (ret != RMW_RET_OK) { @@ -353,6 +407,7 @@ rcl_node_fini(rcl_node_t * node) } allocator.deallocate(node->impl->graph_guard_condition, allocator.state); // assuming that allocate and deallocate are ok since they are checked in init + allocator.deallocate((char *)node->impl->logger_name, allocator.state); allocator.deallocate(node->impl, allocator.state); node->impl = NULL; RCUTILS_LOG_DEBUG_NAMED(ROS_PACKAGE_NAME, "Node finalized") @@ -456,6 +511,15 @@ rcl_node_get_graph_guard_condition(const rcl_node_t * node) return node->impl->graph_guard_condition; } +const char * +rcl_node_get_logger_name(const rcl_node_t * node) +{ + if (!rcl_node_is_valid(node, NULL)) { + return NULL; + } + return node->impl->logger_name; +} + #if __cplusplus } #endif diff --git a/rcl/test/rcl/test_node.cpp b/rcl/test/rcl/test_node.cpp index ad5c3b6..c97284a 100644 --- a/rcl/test/rcl/test_node.cpp +++ b/rcl/test/rcl/test_node.cpp @@ -178,6 +178,30 @@ TEST_F(CLASSNAME(TestNodeFixture, RMW_IMPLEMENTATION), test_rcl_node_accessors) if (actual_node_namespace) { EXPECT_EQ(std::string(namespace_), std::string(actual_node_namespace)); } + // Test rcl_node_get_logger_name(). + const char * actual_node_logger_name; + actual_node_logger_name = rcl_node_get_logger_name(nullptr); + EXPECT_EQ(nullptr, actual_node_logger_name); + rcl_reset_error(); + actual_node_logger_name = rcl_node_get_logger_name(&zero_node); + EXPECT_EQ(nullptr, actual_node_logger_name); + rcl_reset_error(); + actual_node_logger_name = rcl_node_get_logger_name(&invalid_node); + EXPECT_EQ(nullptr, actual_node_logger_name); + rcl_reset_error(); + start_memory_checking(); + assert_no_malloc_begin(); + assert_no_realloc_begin(); + assert_no_free_begin(); + actual_node_logger_name = rcl_node_get_logger_name(&node); + assert_no_malloc_end(); + assert_no_realloc_end(); + assert_no_free_end(); + stop_memory_checking(); + EXPECT_TRUE(actual_node_logger_name ? true : false); + if (actual_node_logger_name) { + EXPECT_EQ("ns." + std::string(name), std::string(actual_node_logger_name)); + } // Test rcl_node_get_options(). const rcl_node_options_t * actual_options; actual_options = rcl_node_get_options(nullptr); @@ -555,3 +579,93 @@ TEST_F(CLASSNAME(TestNodeFixture, RMW_IMPLEMENTATION), test_rcl_node_namespace_r EXPECT_EQ(RCL_RET_OK, ret); } } + +/* Tests the logger name associated with the node. + */ +TEST_F(CLASSNAME(TestNodeFixture, RMW_IMPLEMENTATION), test_rcl_node_logger_name) { + stop_memory_checking(); + rcl_ret_t ret; + + // Initialize rcl with rcl_init(). + ret = rcl_init(0, nullptr, rcl_get_default_allocator()); + ASSERT_EQ(RCL_RET_OK, ret); + auto rcl_shutdown_exit = make_scope_exit( + []() { + rcl_ret_t ret = rcl_shutdown(); + ASSERT_EQ(RCL_RET_OK, ret); + }); + + const char * name = "node"; + const char * actual_node_logger_name; + rcl_node_options_t default_options = rcl_node_get_default_options(); + + // First do a normal node namespace. + { + rcl_node_t node = rcl_get_zero_initialized_node(); + ret = rcl_node_init(&node, name, "/ns", &default_options); + ASSERT_EQ(RCL_RET_OK, ret); + actual_node_logger_name = rcl_node_get_logger_name(&node); + EXPECT_TRUE(actual_node_logger_name ? true : false); + if (actual_node_logger_name) { + EXPECT_EQ("ns." + std::string(name), std::string(actual_node_logger_name)); + } + rcl_ret_t ret = rcl_node_fini(&node); + EXPECT_EQ(RCL_RET_OK, ret); + } + + // Node namespace that is an empty string. + { + rcl_node_t node = rcl_get_zero_initialized_node(); + ret = rcl_node_init(&node, name, "", &default_options); + ASSERT_EQ(RCL_RET_OK, ret); + actual_node_logger_name = rcl_node_get_logger_name(&node); + EXPECT_TRUE(actual_node_logger_name ? true : false); + if (actual_node_logger_name) { + EXPECT_EQ(std::string(name), std::string(actual_node_logger_name)); + } + rcl_ret_t ret = rcl_node_fini(&node); + EXPECT_EQ(RCL_RET_OK, ret); + } + + // Node namespace that is just a forward slash. + { + rcl_node_t node = rcl_get_zero_initialized_node(); + ret = rcl_node_init(&node, name, "/", &default_options); + ASSERT_EQ(RCL_RET_OK, ret); + actual_node_logger_name = rcl_node_get_logger_name(&node); + EXPECT_TRUE(actual_node_logger_name ? true : false); + if (actual_node_logger_name) { + EXPECT_EQ(std::string(name), std::string(actual_node_logger_name)); + } + rcl_ret_t ret = rcl_node_fini(&node); + EXPECT_EQ(RCL_RET_OK, ret); + } + + // Node namespace that is not absolute. + { + rcl_node_t node = rcl_get_zero_initialized_node(); + ret = rcl_node_init(&node, name, "ns", &default_options); + ASSERT_EQ(RCL_RET_OK, ret); + actual_node_logger_name = rcl_node_get_logger_name(&node); + EXPECT_TRUE(actual_node_logger_name ? true : false); + if (actual_node_logger_name) { + EXPECT_EQ("ns." + std::string(name), std::string(actual_node_logger_name)); + } + rcl_ret_t ret = rcl_node_fini(&node); + EXPECT_EQ(RCL_RET_OK, ret); + } + + // Nested namespace. + { + rcl_node_t node = rcl_get_zero_initialized_node(); + ret = rcl_node_init(&node, name, "/ns/sub_1/sub_2", &default_options); + ASSERT_EQ(RCL_RET_OK, ret); + actual_node_logger_name = rcl_node_get_logger_name(&node); + EXPECT_TRUE(actual_node_logger_name ? true : false); + if (actual_node_logger_name) { + EXPECT_EQ("ns.sub_1.sub_2." + std::string(name), std::string(actual_node_logger_name)); + } + rcl_ret_t ret = rcl_node_fini(&node); + EXPECT_EQ(RCL_RET_OK, ret); + } +}