From 8b65abeed5462f00ca446a8948294dc2b4214b65 Mon Sep 17 00:00:00 2001 From: Michel Hidalgo Date: Wed, 31 Oct 2018 15:46:55 -0300 Subject: [PATCH 1/2] [rcl action] Add action services and topics name getters --- rcl_action/CMakeLists.txt | 13 +- rcl_action/include/rcl_action/names.h | 194 ++++++++++++++++++++++ rcl_action/src/rcl_action/names.c | 112 +++++++++++++ rcl_action/test/rcl_action/test_names.cpp | 91 ++++++++++ 4 files changed, 409 insertions(+), 1 deletion(-) create mode 100644 rcl_action/include/rcl_action/names.h create mode 100644 rcl_action/src/rcl_action/names.c create mode 100644 rcl_action/test/rcl_action/test_names.cpp diff --git a/rcl_action/CMakeLists.txt b/rcl_action/CMakeLists.txt index feb70a8..55d9cf8 100644 --- a/rcl_action/CMakeLists.txt +++ b/rcl_action/CMakeLists.txt @@ -33,6 +33,7 @@ add_executable(test_compile_headers set(rcl_action_sources src/${PROJECT_NAME}/goal_state_machine.c + src/${PROJECT_NAME}/names.c src/${PROJECT_NAME}/types.c ) @@ -95,6 +96,16 @@ if(BUILD_TESTING) target_link_libraries(test_types ${PROJECT_NAME} ) + ament_add_gtest(test_names + test/rcl_action/test_names.cpp + ) + if(TARGET test_names) + target_include_directories(test_names PUBLIC + ${rcl_INCLUDE_DIRS} + ) + target_link_libraries(test_names + ${PROJECT_NAME} + ) endif() endif() @@ -104,4 +115,4 @@ ament_export_libraries(${PROJECT_NAME}) ament_export_dependencies(ament_cmake) ament_export_dependencies(rcl) ament_export_dependencies(action_msgs) -ament_package() \ No newline at end of file +ament_package() diff --git a/rcl_action/include/rcl_action/names.h b/rcl_action/include/rcl_action/names.h new file mode 100644 index 0000000..5fa2191 --- /dev/null +++ b/rcl_action/include/rcl_action/names.h @@ -0,0 +1,194 @@ +// Copyright 2018 Open Source Robotics Foundation, Inc. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +#ifndef RCL_ACTION__NAMES_H_ +#define RCL_ACTION__NAMES_H_ + +#ifdef __cplusplus +extern "C" +{ +#endif + +#include "rcl_action/visibility_control.h" + +#include "rcl/allocator.h" +#include "rcl/macros.h" +#include "rcl/types.h" + + +/// Get the goal service name of an action. +/** + * This function returns the goal service name for a given action name + * that must be used by action clients and action servers to successfully + * communicate with each other. + * + *
+ * Attribute | Adherence + * ------------------ | ------------- + * Allocates Memory | Yes + * Thread-Safe | No + * Uses Atomics | No + * Lock-Free | Yes + * + * \param[in] action_name The name of the action whose goal service name is + * being returned. + * \param[in] allocator A valid allocator to be used. + * \param[out] goal_service_name Either an allocated string with the action + * goal service name, or `NULL` if the function failed to allocate memory + * for it. + * \return `RCL_RET_OK` if the action goal service name was returned, or + * \return `RCL_RET_INVALID_ARGUMENT` if any arguments are invalid, or + * \return `RCL_RET_BAD_ALLOC` if allocating memory failed. + */ +RCL_ACTION_PUBLIC +RCL_WARN_UNUSED +rcl_ret_t +rcl_action_get_goal_service_name( + const char * action_name, + rcl_allocator_t allocator, + char ** goal_service_name); + +/// Get the cancel service name of an action. +/** + * This function returns the cancel service name for a given action name + * that must be used by action clients and action servers to successfully + * communicate with each other. + * + *
+ * Attribute | Adherence + * ------------------ | ------------- + * Allocates Memory | Yes + * Thread-Safe | No + * Uses Atomics | No + * Lock-Free | Yes + * + * \param[in] action_name The name of the action whose cancel service name is + * being returned. + * \param[in] allocator A valid allocator to be used. + * \param[out] cancel_service_name Either an allocated string with the action + * cancel service name, or `NULL` if the function failed to allocate memory + * for it. + * \return `RCL_RET_OK` if the action cancel service name was returned, or + * \return `RCL_RET_INVALID_ARGUMENT` if any arguments are invalid, or + * \return `RCL_RET_BAD_ALLOC` if allocating memory failed. + */ +RCL_ACTION_PUBLIC +RCL_WARN_UNUSED +rcl_ret_t +rcl_action_get_cancel_service_name( + const char * action_name, + rcl_allocator_t allocator, + char ** cancel_service_name); + +/// Get the result service name of an action. +/** + * This function returns the result service name for a given action name + * that must be used by action clients and action servers to successfully + * communicate with each other. + * + *
+ * Attribute | Adherence + * ------------------ | ------------- + * Allocates Memory | Yes + * Thread-Safe | No + * Uses Atomics | No + * Lock-Free | Yes + * + * \param[in] action_name The name of the action whose result service name is + * being returned. + * \param[in] allocator A valid allocator to be used. + * \param[out] result_service_name Either an allocated string with the action + * result service name, or `NULL` if the function failed to allocate memory + * for it. + * \return `RCL_RET_OK` if the action result service name was returned, or + * \return `RCL_RET_INVALID_ARGUMENT` if any arguments are invalid, or + * \return `RCL_RET_BAD_ALLOC` if allocating memory failed. + */ +RCL_ACTION_PUBLIC +RCL_WARN_UNUSED +rcl_ret_t +rcl_action_get_result_service_name( + const char * action_name, + rcl_allocator_t allocator, + char ** result_service_name); + +/// Get the feedback topic name of an action. +/** + * This function returns the feedback topic name for a given action name + * that must be used by action clients and action servers to successfully + * communicate with each other. + * + *
+ * Attribute | Adherence + * ------------------ | ------------- + * Allocates Memory | Yes + * Thread-Safe | No + * Uses Atomics | No + * Lock-Free | Yes + * + * \param[in] action_name The name of the action whose feedback topic name is + * being returned. + * \param[in] allocator A valid allocator to be used. + * \param[out] result_service_name Either an allocated string with the action + * feedback topic name, or `NULL` if the function failed to allocate memory + * for it. + * \return `RCL_RET_OK` if the action feedback topic name was returned, or + * \return `RCL_RET_INVALID_ARGUMENT` if any arguments are invalid, or + * \return `RCL_RET_BAD_ALLOC` if allocating memory failed. + */ +RCL_ACTION_PUBLIC +RCL_WARN_UNUSED +rcl_ret_t +rcl_action_get_feedback_topic_name( + const char * action_name, + rcl_allocator_t allocator, + char ** feedback_topic_name); + +/// Get the status topic name of an action. +/** + * This function returns the status topic name for a given action name + * that must be used by action clients and action servers to successfully + * communicate with each other. + * + *
+ * Attribute | Adherence + * ------------------ | ------------- + * Allocates Memory | Yes + * Thread-Safe | No + * Uses Atomics | No + * Lock-Free | Yes + * + * \param[in] action_name The name of the action whose status topic name is + * being returned. + * \param[in] allocator A valid allocator to be used. + * \param[out] result_service_name Either an allocated string with the action + * status topic name, or `NULL` if the function failed to allocate memory + * for it. + * \return `RCL_RET_OK` if the action status topic name was returned, or + * \return `RCL_RET_INVALID_ARGUMENT` if any arguments are invalid, or + * \return `RCL_RET_BAD_ALLOC` if allocating memory failed. + */ +RCL_ACTION_PUBLIC +RCL_WARN_UNUSED +rcl_ret_t +rcl_action_get_status_topic_name( + const char * action_name, + rcl_allocator_t allocator, + char ** status_topic_name); + +#ifdef __cplusplus +} +#endif + +#endif // RCL_ACTION__NAMES_H_ diff --git a/rcl_action/src/rcl_action/names.c b/rcl_action/src/rcl_action/names.c new file mode 100644 index 0000000..4185541 --- /dev/null +++ b/rcl_action/src/rcl_action/names.c @@ -0,0 +1,112 @@ +// Copyright 2018 Open Source Robotics Foundation, Inc. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +#ifdef __cplusplus +extern "C" +{ +#endif + +#include "rcl_action/names.h" + +#include "rcl/error_handling.h" +#include "rcutils/format_string.h" + +rcl_ret_t +rcl_action_get_goal_service_name( + const char * action_name, + rcl_allocator_t allocator, + char ** goal_service_name) +{ + RCL_CHECK_ALLOCATOR_WITH_MSG(&allocator, "allocator is invalid", return RCL_RET_INVALID_ARGUMENT); + RCL_CHECK_ARGUMENT_FOR_NULL(action_name, RCL_RET_INVALID_ARGUMENT, allocator); + RCL_CHECK_ARGUMENT_FOR_NULL(goal_service_name, RCL_RET_INVALID_ARGUMENT, allocator); + *goal_service_name = rcutils_format_string(allocator, "%s/_action/send_goal", action_name); + if (*goal_service_name == NULL) { + RCL_SET_ERROR_MSG("failed to allocate memory for action goal service name", allocator); + return RCL_RET_BAD_ALLOC; + } + return RCL_RET_OK; +} + +rcl_ret_t +rcl_action_get_cancel_service_name( + const char * action_name, + rcl_allocator_t allocator, + char ** cancel_service_name) +{ + RCL_CHECK_ALLOCATOR_WITH_MSG(&allocator, "allocator is invalid", return RCL_RET_INVALID_ARGUMENT); + RCL_CHECK_ARGUMENT_FOR_NULL(action_name, RCL_RET_INVALID_ARGUMENT, allocator); + RCL_CHECK_ARGUMENT_FOR_NULL(cancel_service_name, RCL_RET_INVALID_ARGUMENT, allocator); + *cancel_service_name = rcutils_format_string(allocator, "%s/_action/cancel_goal", action_name); + if (*cancel_service_name == NULL) { + RCL_SET_ERROR_MSG("failed to allocate memory for action cancel service name", allocator); + return RCL_RET_BAD_ALLOC; + } + return RCL_RET_OK; +} + +rcl_ret_t +rcl_action_get_result_service_name( + const char * action_name, + rcl_allocator_t allocator, + char ** result_service_name) +{ + RCL_CHECK_ALLOCATOR_WITH_MSG(&allocator, "allocator is invalid", return RCL_RET_INVALID_ARGUMENT); + RCL_CHECK_ARGUMENT_FOR_NULL(action_name, RCL_RET_INVALID_ARGUMENT, allocator); + RCL_CHECK_ARGUMENT_FOR_NULL(result_service_name, RCL_RET_INVALID_ARGUMENT, allocator); + *result_service_name = rcutils_format_string(allocator, "%s/_action/get_result", action_name); + if (*result_service_name == NULL) { + RCL_SET_ERROR_MSG("failed to allocate memory for action result service name", allocator); + return RCL_RET_BAD_ALLOC; + } + return RCL_RET_OK; +} + +rcl_ret_t +rcl_action_get_feedback_topic_name( + const char * action_name, + rcl_allocator_t allocator, + char ** feedback_topic_name) +{ + RCL_CHECK_ALLOCATOR_WITH_MSG(&allocator, "allocator is invalid", return RCL_RET_INVALID_ARGUMENT); + RCL_CHECK_ARGUMENT_FOR_NULL(action_name, RCL_RET_INVALID_ARGUMENT, allocator); + RCL_CHECK_ARGUMENT_FOR_NULL(feedback_topic_name, RCL_RET_INVALID_ARGUMENT, allocator); + *feedback_topic_name = rcutils_format_string(allocator, "%s/_action/feedback", action_name); + if (*feedback_topic_name == NULL) { + RCL_SET_ERROR_MSG("failed to allocate memory for action feedback topic name", allocator); + return RCL_RET_BAD_ALLOC; + } + return RCL_RET_OK; +} + +rcl_ret_t +rcl_action_get_status_topic_name( + const char * action_name, + rcl_allocator_t allocator, + char ** status_topic_name) +{ + RCL_CHECK_ALLOCATOR_WITH_MSG(&allocator, "allocator is invalid", return RCL_RET_INVALID_ARGUMENT); + RCL_CHECK_ARGUMENT_FOR_NULL(action_name, RCL_RET_INVALID_ARGUMENT, allocator); + RCL_CHECK_ARGUMENT_FOR_NULL(status_topic_name, RCL_RET_INVALID_ARGUMENT, allocator); + *status_topic_name = rcutils_format_string(allocator, "%s/_action/status", action_name); + if (*status_topic_name == NULL) { + RCL_SET_ERROR_MSG("failed to allocate memory for action status topic name", allocator); + return RCL_RET_BAD_ALLOC; + } + return RCL_RET_OK; +} + +#ifdef __cplusplus +} +#endif diff --git a/rcl_action/test/rcl_action/test_names.cpp b/rcl_action/test/rcl_action/test_names.cpp new file mode 100644 index 0000000..0581b20 --- /dev/null +++ b/rcl_action/test/rcl_action/test_names.cpp @@ -0,0 +1,91 @@ +// Copyright 2018 Open Source Robotics Foundation, Inc. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +#include + +#include "rcl_action/names.h" + +#include "rcl/allocator.h" +#include "rcl/types.h" + + +TEST(TestActionNames, test_action_goal_service_name) +{ + rcl_ret_t ret; + const char * const action_name = "test_it"; + rcl_allocator_t default_allocator = rcl_get_default_allocator(); + + char * action_goal_service_name; + ret = rcl_action_get_goal_service_name( + action_name, default_allocator, &action_goal_service_name); + ASSERT_EQ(RCL_RET_OK, ret); + EXPECT_STREQ("test_it/_action/send_goal", action_goal_service_name); + default_allocator.deallocate(action_goal_service_name, default_allocator.state); +} + +TEST(TestActionNames, test_action_cancel_service_name) +{ + rcl_ret_t ret; + const char * const action_name = "test_it"; + rcl_allocator_t default_allocator = rcl_get_default_allocator(); + + char * action_cancel_service_name; + ret = rcl_action_get_cancel_service_name( + action_name, default_allocator, &action_cancel_service_name); + ASSERT_EQ(RCL_RET_OK, ret); + EXPECT_STREQ("test_it/_action/cancel_goal", action_cancel_service_name); + default_allocator.deallocate(action_cancel_service_name, default_allocator.state); +} + +TEST(TestActionNames, test_action_result_service_name) +{ + rcl_ret_t ret; + const char * const action_name = "test_it"; + rcl_allocator_t default_allocator = rcl_get_default_allocator(); + + char * action_result_service_name; + ret = rcl_action_get_result_service_name( + action_name, default_allocator, &action_result_service_name); + ASSERT_EQ(RCL_RET_OK, ret); + EXPECT_STREQ("test_it/_action/get_result", action_result_service_name); + default_allocator.deallocate(action_result_service_name, default_allocator.state); +} + +TEST(TestActionNames, test_action_feedback_topic_name) +{ + rcl_ret_t ret; + const char * const action_name = "test_it"; + rcl_allocator_t default_allocator = rcl_get_default_allocator(); + + char * action_feedback_topic_name; + ret = rcl_action_get_feedback_topic_name( + action_name, default_allocator, &action_feedback_topic_name); + ASSERT_EQ(RCL_RET_OK, ret); + EXPECT_STREQ("test_it/_action/feedback", action_feedback_topic_name); + default_allocator.deallocate(action_feedback_topic_name, default_allocator.state); +} + +TEST(TestActionNames, test_action_status_topic_name) +{ + rcl_ret_t ret; + const char * const action_name = "test_it"; + rcl_allocator_t default_allocator = rcl_get_default_allocator(); + + char * action_status_topic_name; + ret = rcl_action_get_status_topic_name( + action_name, default_allocator, &action_status_topic_name); + ASSERT_EQ(RCL_RET_OK, ret); + EXPECT_STREQ("test_it/_action/status", action_status_topic_name); + default_allocator.deallocate(action_status_topic_name, default_allocator.state); +} From 55d83f27d785b8dbc2028c4ce88d268c103afd11 Mon Sep 17 00:00:00 2001 From: Michel Hidalgo Date: Thu, 1 Nov 2018 10:52:42 -0300 Subject: [PATCH 2/2] [rcl action] Addresses peer review comments (#317) --- rcl_action/CMakeLists.txt | 1 + rcl_action/include/rcl_action/names.h | 10 +- rcl_action/src/rcl_action/names.c | 31 ++++- rcl_action/test/rcl_action/test_names.cpp | 157 ++++++++++++++-------- 4 files changed, 130 insertions(+), 69 deletions(-) diff --git a/rcl_action/CMakeLists.txt b/rcl_action/CMakeLists.txt index 55d9cf8..4ae684e 100644 --- a/rcl_action/CMakeLists.txt +++ b/rcl_action/CMakeLists.txt @@ -96,6 +96,7 @@ if(BUILD_TESTING) target_link_libraries(test_types ${PROJECT_NAME} ) + endif() ament_add_gtest(test_names test/rcl_action/test_names.cpp ) diff --git a/rcl_action/include/rcl_action/names.h b/rcl_action/include/rcl_action/names.h index 5fa2191..411fc61 100644 --- a/rcl_action/include/rcl_action/names.h +++ b/rcl_action/include/rcl_action/names.h @@ -46,7 +46,7 @@ extern "C" * \param[in] allocator A valid allocator to be used. * \param[out] goal_service_name Either an allocated string with the action * goal service name, or `NULL` if the function failed to allocate memory - * for it. + * for it. Must refer to a `NULL` pointer upon call. * \return `RCL_RET_OK` if the action goal service name was returned, or * \return `RCL_RET_INVALID_ARGUMENT` if any arguments are invalid, or * \return `RCL_RET_BAD_ALLOC` if allocating memory failed. @@ -78,7 +78,7 @@ rcl_action_get_goal_service_name( * \param[in] allocator A valid allocator to be used. * \param[out] cancel_service_name Either an allocated string with the action * cancel service name, or `NULL` if the function failed to allocate memory - * for it. + * for it. Must refer to a `NULL` pointer upon call. * \return `RCL_RET_OK` if the action cancel service name was returned, or * \return `RCL_RET_INVALID_ARGUMENT` if any arguments are invalid, or * \return `RCL_RET_BAD_ALLOC` if allocating memory failed. @@ -110,7 +110,7 @@ rcl_action_get_cancel_service_name( * \param[in] allocator A valid allocator to be used. * \param[out] result_service_name Either an allocated string with the action * result service name, or `NULL` if the function failed to allocate memory - * for it. + * for it. Must refer to a `NULL` pointer upon call. * \return `RCL_RET_OK` if the action result service name was returned, or * \return `RCL_RET_INVALID_ARGUMENT` if any arguments are invalid, or * \return `RCL_RET_BAD_ALLOC` if allocating memory failed. @@ -142,7 +142,7 @@ rcl_action_get_result_service_name( * \param[in] allocator A valid allocator to be used. * \param[out] result_service_name Either an allocated string with the action * feedback topic name, or `NULL` if the function failed to allocate memory - * for it. + * for it. Must refer to a `NULL` pointer upon call. * \return `RCL_RET_OK` if the action feedback topic name was returned, or * \return `RCL_RET_INVALID_ARGUMENT` if any arguments are invalid, or * \return `RCL_RET_BAD_ALLOC` if allocating memory failed. @@ -174,7 +174,7 @@ rcl_action_get_feedback_topic_name( * \param[in] allocator A valid allocator to be used. * \param[out] result_service_name Either an allocated string with the action * status topic name, or `NULL` if the function failed to allocate memory - * for it. + * for it. Must refer to a `NULL` pointer upon call. * \return `RCL_RET_OK` if the action status topic name was returned, or * \return `RCL_RET_INVALID_ARGUMENT` if any arguments are invalid, or * \return `RCL_RET_BAD_ALLOC` if allocating memory failed. diff --git a/rcl_action/src/rcl_action/names.c b/rcl_action/src/rcl_action/names.c index 4185541..6ba3c92 100644 --- a/rcl_action/src/rcl_action/names.c +++ b/rcl_action/src/rcl_action/names.c @@ -31,8 +31,12 @@ rcl_action_get_goal_service_name( RCL_CHECK_ALLOCATOR_WITH_MSG(&allocator, "allocator is invalid", return RCL_RET_INVALID_ARGUMENT); RCL_CHECK_ARGUMENT_FOR_NULL(action_name, RCL_RET_INVALID_ARGUMENT, allocator); RCL_CHECK_ARGUMENT_FOR_NULL(goal_service_name, RCL_RET_INVALID_ARGUMENT, allocator); + if (NULL != *goal_service_name) { + RCL_SET_ERROR_MSG("writing action goal service name may leak memory", allocator); + return RCL_RET_INVALID_ARGUMENT; + } *goal_service_name = rcutils_format_string(allocator, "%s/_action/send_goal", action_name); - if (*goal_service_name == NULL) { + if (NULL == *goal_service_name) { RCL_SET_ERROR_MSG("failed to allocate memory for action goal service name", allocator); return RCL_RET_BAD_ALLOC; } @@ -48,8 +52,12 @@ rcl_action_get_cancel_service_name( RCL_CHECK_ALLOCATOR_WITH_MSG(&allocator, "allocator is invalid", return RCL_RET_INVALID_ARGUMENT); RCL_CHECK_ARGUMENT_FOR_NULL(action_name, RCL_RET_INVALID_ARGUMENT, allocator); RCL_CHECK_ARGUMENT_FOR_NULL(cancel_service_name, RCL_RET_INVALID_ARGUMENT, allocator); + if (NULL != *cancel_service_name) { + RCL_SET_ERROR_MSG("writing action cancel service name may leak memory", allocator); + return RCL_RET_INVALID_ARGUMENT; + } *cancel_service_name = rcutils_format_string(allocator, "%s/_action/cancel_goal", action_name); - if (*cancel_service_name == NULL) { + if (NULL == *cancel_service_name) { RCL_SET_ERROR_MSG("failed to allocate memory for action cancel service name", allocator); return RCL_RET_BAD_ALLOC; } @@ -65,8 +73,13 @@ rcl_action_get_result_service_name( RCL_CHECK_ALLOCATOR_WITH_MSG(&allocator, "allocator is invalid", return RCL_RET_INVALID_ARGUMENT); RCL_CHECK_ARGUMENT_FOR_NULL(action_name, RCL_RET_INVALID_ARGUMENT, allocator); RCL_CHECK_ARGUMENT_FOR_NULL(result_service_name, RCL_RET_INVALID_ARGUMENT, allocator); + if (NULL != *result_service_name) { + RCL_SET_ERROR_MSG("writing action result service name may leak memory", allocator); + return RCL_RET_INVALID_ARGUMENT; + } + *result_service_name = rcutils_format_string(allocator, "%s/_action/get_result", action_name); - if (*result_service_name == NULL) { + if (NULL == *result_service_name) { RCL_SET_ERROR_MSG("failed to allocate memory for action result service name", allocator); return RCL_RET_BAD_ALLOC; } @@ -82,8 +95,12 @@ rcl_action_get_feedback_topic_name( RCL_CHECK_ALLOCATOR_WITH_MSG(&allocator, "allocator is invalid", return RCL_RET_INVALID_ARGUMENT); RCL_CHECK_ARGUMENT_FOR_NULL(action_name, RCL_RET_INVALID_ARGUMENT, allocator); RCL_CHECK_ARGUMENT_FOR_NULL(feedback_topic_name, RCL_RET_INVALID_ARGUMENT, allocator); + if (NULL != *feedback_topic_name) { + RCL_SET_ERROR_MSG("writing action feedback topic name may leak memory", allocator); + return RCL_RET_INVALID_ARGUMENT; + } *feedback_topic_name = rcutils_format_string(allocator, "%s/_action/feedback", action_name); - if (*feedback_topic_name == NULL) { + if (NULL == *feedback_topic_name) { RCL_SET_ERROR_MSG("failed to allocate memory for action feedback topic name", allocator); return RCL_RET_BAD_ALLOC; } @@ -99,8 +116,12 @@ rcl_action_get_status_topic_name( RCL_CHECK_ALLOCATOR_WITH_MSG(&allocator, "allocator is invalid", return RCL_RET_INVALID_ARGUMENT); RCL_CHECK_ARGUMENT_FOR_NULL(action_name, RCL_RET_INVALID_ARGUMENT, allocator); RCL_CHECK_ARGUMENT_FOR_NULL(status_topic_name, RCL_RET_INVALID_ARGUMENT, allocator); + if (NULL != *status_topic_name) { + RCL_SET_ERROR_MSG("writing action status topic name may leak memory", allocator); + return RCL_RET_INVALID_ARGUMENT; + } *status_topic_name = rcutils_format_string(allocator, "%s/_action/status", action_name); - if (*status_topic_name == NULL) { + if (NULL == *status_topic_name) { RCL_SET_ERROR_MSG("failed to allocate memory for action status topic name", allocator); return RCL_RET_BAD_ALLOC; } diff --git a/rcl_action/test/rcl_action/test_names.cpp b/rcl_action/test/rcl_action/test_names.cpp index 0581b20..da9e3fa 100644 --- a/rcl_action/test/rcl_action/test_names.cpp +++ b/rcl_action/test/rcl_action/test_names.cpp @@ -14,78 +14,117 @@ #include +#include + #include "rcl_action/names.h" #include "rcl/allocator.h" #include "rcl/types.h" - -TEST(TestActionNames, test_action_goal_service_name) +struct ActionDerivedNameTestSubject { - rcl_ret_t ret; - const char * const action_name = "test_it"; - rcl_allocator_t default_allocator = rcl_get_default_allocator(); + const char * action_name; + const char * expected_action_derived_name; + rcl_ret_t (* get_action_derived_name)(const char *, rcl_allocator_t, char **); + const char * subject_name; +}; - char * action_goal_service_name; - ret = rcl_action_get_goal_service_name( - action_name, default_allocator, &action_goal_service_name); - ASSERT_EQ(RCL_RET_OK, ret); - EXPECT_STREQ("test_it/_action/send_goal", action_goal_service_name); - default_allocator.deallocate(action_goal_service_name, default_allocator.state); +std::ostream & operator<<(std::ostream & os, const ActionDerivedNameTestSubject & test_subject) +{ + return os << test_subject.subject_name; } -TEST(TestActionNames, test_action_cancel_service_name) +class TestActionDerivedName + : public ::testing::TestWithParam +{ +protected: + void SetUp() override + { + test_subject = GetParam(); + } + + ActionDerivedNameTestSubject test_subject; +}; + +TEST_P(TestActionDerivedName, validate_action_derived_getter) { rcl_ret_t ret; - const char * const action_name = "test_it"; - rcl_allocator_t default_allocator = rcl_get_default_allocator(); + char dummy_char; + char * action_derived_name; + rcl_allocator_t default_allocator = + rcl_get_default_allocator(); - char * action_cancel_service_name; - ret = rcl_action_get_cancel_service_name( - action_name, default_allocator, &action_cancel_service_name); + action_derived_name = NULL; + const char * const invalid_action_name = NULL; + ret = test_subject.get_action_derived_name( + invalid_action_name, default_allocator, + &action_derived_name); + EXPECT_EQ(RCL_RET_INVALID_ARGUMENT, ret); + + action_derived_name = NULL; + rcl_allocator_t invalid_allocator = + (rcl_allocator_t)rcutils_get_zero_initialized_allocator(); + ret = test_subject.get_action_derived_name( + test_subject.action_name, invalid_allocator, + &action_derived_name); + EXPECT_EQ(RCL_RET_INVALID_ARGUMENT, ret); + + action_derived_name = NULL; + char ** invalid_ptr_to_action_derived_name = NULL; + ret = test_subject.get_action_derived_name( + test_subject.action_name, default_allocator, + invalid_ptr_to_action_derived_name); + EXPECT_EQ(RCL_RET_INVALID_ARGUMENT, ret); + + action_derived_name = &dummy_char; + ret = test_subject.get_action_derived_name( + test_subject.action_name, default_allocator, + &action_derived_name); + EXPECT_EQ(RCL_RET_INVALID_ARGUMENT, ret); + + action_derived_name = NULL; + ret = test_subject.get_action_derived_name( + test_subject.action_name, default_allocator, + &action_derived_name); ASSERT_EQ(RCL_RET_OK, ret); - EXPECT_STREQ("test_it/_action/cancel_goal", action_cancel_service_name); - default_allocator.deallocate(action_cancel_service_name, default_allocator.state); + EXPECT_STREQ(test_subject.expected_action_derived_name, action_derived_name); + default_allocator.deallocate(action_derived_name, default_allocator.state); } -TEST(TestActionNames, test_action_result_service_name) -{ - rcl_ret_t ret; - const char * const action_name = "test_it"; - rcl_allocator_t default_allocator = rcl_get_default_allocator(); +const ActionDerivedNameTestSubject action_service_and_topic_subjects[] = { + { + "test_it", // action_name + "test_it/_action/send_goal", // expected_action_derived_name + rcl_action_get_goal_service_name, // get_action_derived_name + "goal_service_name_test" // subject_name + }, + { + "test_it", // action_name + "test_it/_action/cancel_goal", // expected_action_derived_name + rcl_action_get_cancel_service_name, // get_action_derived_name + "cancel_service_name_test" // subject_name + }, + { + "test_it", // action_name + "test_it/_action/get_result", // expected_action_derived_name + rcl_action_get_result_service_name, // get_action_derived_name + "result_service_name_test" // subject_name + }, + { + "test_it", // action_name + "test_it/_action/feedback", // expected_action_derived_name + rcl_action_get_feedback_topic_name, // get_action_derived_name + "feedback_topic_name_test" // subject_name + }, + { + "test_it", // action_name + "test_it/_action/status", // expected_action_derived_name + rcl_action_get_status_topic_name, // get_action_derived_name + "status_topic_name_test" // subject_name + } +}; - char * action_result_service_name; - ret = rcl_action_get_result_service_name( - action_name, default_allocator, &action_result_service_name); - ASSERT_EQ(RCL_RET_OK, ret); - EXPECT_STREQ("test_it/_action/get_result", action_result_service_name); - default_allocator.deallocate(action_result_service_name, default_allocator.state); -} - -TEST(TestActionNames, test_action_feedback_topic_name) -{ - rcl_ret_t ret; - const char * const action_name = "test_it"; - rcl_allocator_t default_allocator = rcl_get_default_allocator(); - - char * action_feedback_topic_name; - ret = rcl_action_get_feedback_topic_name( - action_name, default_allocator, &action_feedback_topic_name); - ASSERT_EQ(RCL_RET_OK, ret); - EXPECT_STREQ("test_it/_action/feedback", action_feedback_topic_name); - default_allocator.deallocate(action_feedback_topic_name, default_allocator.state); -} - -TEST(TestActionNames, test_action_status_topic_name) -{ - rcl_ret_t ret; - const char * const action_name = "test_it"; - rcl_allocator_t default_allocator = rcl_get_default_allocator(); - - char * action_status_topic_name; - ret = rcl_action_get_status_topic_name( - action_name, default_allocator, &action_status_topic_name); - ASSERT_EQ(RCL_RET_OK, ret); - EXPECT_STREQ("test_it/_action/status", action_status_topic_name); - default_allocator.deallocate(action_status_topic_name, default_allocator.state); -} +INSTANTIATE_TEST_CASE_P( + TestActionServiceAndTopicNames, TestActionDerivedName, + ::testing::ValuesIn(action_service_and_topic_subjects), + ::testing::PrintToStringParamName());