niklas/rcl
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

Topic fix rcl lifecycle test issue (#715) (#796)

Browse source 
* Fix missing call fini() for lifecycle_transition and node in test_rcl_lifecycle

Signed-off-by: Barry Xu <barry.xu@sony.com>

* Fix error overwritten while allocator is Nullptr.

Signed-off-by: Barry Xu <barry.xu@sony.com>

* Optimize used variables

Signed-off-by: Barry Xu <barry.xu@sony.com>
Signed-off-by: Shane Loretz <sloretz@osrfoundation.org>

Co-authored-by: Barry Xu <barry.xu@sony.com>
This commit is contained in:
Shane Loretz 2020-09-09 15:25:10 -07:00 • committed by GitHub
parent 474754586c
commit 055d7eba62
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
2 changed files with 52 additions and 20 deletions
Download patch file Download diff file Expand all files Collapse all files

32
rcl_lifecycle/src/default_state_machine.c
View file

@ -23,6 +23,7 @@
#include "rcl/error_handling.h" #include "rcl/error_handling.h"
#include "rcl/rcl.h" #include "rcl/rcl.h"
#include "rcutils/strdup.h"
#include "rcl_lifecycle/transition_map.h" #include "rcl_lifecycle/transition_map.h"
@ -658,7 +659,11 @@ rcl_lifecycle_init_default_state_machine(
{ {
rcl_ret_t ret = RCL_RET_ERROR; rcl_ret_t ret = RCL_RET_ERROR;
// Used for concatenating error messages in the fail: block. // Used for concatenating error messages in the fail: block.
const char * fail_error_message = ""; // The cause or error which leads to jump to fail:
char * fail_error_message = NULL;
// The error happens in fail:
char * fini_error_message = NULL;
rcl_allocator_t default_allocator;
// *************************** // ***************************
// register all primary states // register all primary states
@ -695,25 +700,40 @@ rcl_lifecycle_init_default_state_machine(
fail: fail:
// If rcl_lifecycle_transition_map_fini() fails, it will clobber the error string here. // If rcl_lifecycle_transition_map_fini() fails, it will clobber the error string here.
// Concatenate the error strings if that happens // Concatenate the error strings if that happens
default_allocator = rcl_get_default_allocator();
if (rcl_error_is_set()) { if (rcl_error_is_set()) {
fail_error_message = rcl_get_error_string().str; fail_error_message = rcutils_strdup(rcl_get_error_string().str, default_allocator);
rcl_reset_error();
} }
if (rcl_lifecycle_transition_map_fini(&state_machine->transition_map, allocator) != RCL_RET_OK) { if (rcl_lifecycle_transition_map_fini(&state_machine->transition_map, allocator) != RCL_RET_OK) {
const char * fini_error = "";
if (rcl_error_is_set()) { if (rcl_error_is_set()) {
fini_error = rcl_get_error_string().str; fini_error_message = rcutils_strdup(rcl_get_error_string().str, default_allocator);
rcl_reset_error(); rcl_reset_error();
} }
RCL_SET_ERROR_MSG_WITH_FORMAT_STRING( RCL_SET_ERROR_MSG_WITH_FORMAT_STRING(
"Freeing transition map failed while handling a previous error. Leaking memory!" "Freeing transition map failed while handling a previous error. Leaking memory!"
"\nOriginal error:\n\t%s\nError encountered in rcl_lifecycle_transition_map_fini():\n\t%s\n", "\nOriginal error:\n\t%s\nError encountered in rcl_lifecycle_transition_map_fini():\n\t%s\n",
fail_error_message, fini_error); fail_error_message != NULL ?
fail_error_message : "Failed to duplicate error while init state machine !",
fini_error_message != NULL ?
fini_error_message : "Failed to duplicate error while fini transition map !");
} }
if (!rcl_error_is_set()) { if (!rcl_error_is_set()) {
RCL_SET_ERROR_MSG("Unspecified error in default_state_machine _register_transitions()"); RCL_SET_ERROR_MSG(
(fail_error_message != NULL) ?
fail_error_message : "Unspecified error in rcl_lifecycle_init_default_state_machine() !");
} }
if (fail_error_message != NULL) {
default_allocator.deallocate(fail_error_message, default_allocator.state);
}
if (fini_error_message != NULL) {
default_allocator.deallocate(fini_error_message, default_allocator.state);
}
return RCL_RET_ERROR; return RCL_RET_ERROR;
} }

40
rcl_lifecycle/test/test_rcl_lifecycle.cpp
View file

@ -133,14 +133,23 @@ TEST(TestRclLifecycle, lifecycle_transition) {
ret = rcl_lifecycle_transition_init( ret = rcl_lifecycle_transition_init(
&transition, expected_id, &expected_label[0], nullptr, nullptr, &allocator); &transition, expected_id, &expected_label[0], nullptr, nullptr, &allocator);
EXPECT_EQ(ret, RCL_RET_OK); EXPECT_EQ(ret, RCL_RET_OK) << rcl_get_error_string().str;
rcutils_reset_error();
ret = rcl_lifecycle_transition_fini(&transition, &allocator);
EXPECT_EQ(ret, RCL_RET_OK) << rcl_get_error_string().str;
rcutils_reset_error(); rcutils_reset_error();
ret = rcl_lifecycle_transition_init( ret = rcl_lifecycle_transition_init(
&transition, expected_id, &expected_label[0], start, nullptr, &allocator); &transition, expected_id, &expected_label[0], start, nullptr, &allocator);
EXPECT_EQ(ret, RCL_RET_OK); EXPECT_EQ(ret, RCL_RET_OK) << rcl_get_error_string().str;
rcutils_reset_error();
ret = rcl_lifecycle_transition_fini(&transition, &allocator);
EXPECT_EQ(ret, RCL_RET_OK) << rcl_get_error_string().str;
rcutils_reset_error(); rcutils_reset_error();
start = reinterpret_cast<rcl_lifecycle_state_t *>(
allocator.allocate(sizeof(rcl_lifecycle_state_t), allocator.state));
*start = rcl_lifecycle_get_zero_initialized_state();
rcl_allocator_t bad_allocator = rcl_get_default_allocator(); rcl_allocator_t bad_allocator = rcl_get_default_allocator();
bad_allocator.allocate = bad_malloc; bad_allocator.allocate = bad_malloc;
bad_allocator.reallocate = bad_realloc; bad_allocator.reallocate = bad_realloc;
@ -190,15 +199,16 @@ TEST(TestRclLifecycle, state_machine) {
ret = rcl_init(0, nullptr, &init_options, &context); ret = rcl_init(0, nullptr, &init_options, &context);
EXPECT_EQ(ret, RCL_RET_OK) << rcl_get_error_string().str; EXPECT_EQ(ret, RCL_RET_OK) << rcl_get_error_string().str;
OSRF_TESTING_TOOLS_CPP_SCOPE_EXIT(
{
ASSERT_EQ(RCL_RET_OK, rcl_shutdown(&context));
ASSERT_EQ(RCL_RET_OK, rcl_context_fini(&context));
});
ret = rcl_node_init(&node, "node", "namespace", &context, &options); ret = rcl_node_init(&node, "node", "namespace", &context, &options);
EXPECT_EQ(ret, RCL_RET_OK) << rcl_get_error_string().str; EXPECT_EQ(ret, RCL_RET_OK) << rcl_get_error_string().str;
OSRF_TESTING_TOOLS_CPP_SCOPE_EXIT(
{
ASSERT_EQ(RCL_RET_OK, rcl_node_fini(&node)) << rcl_get_error_string().str;
ASSERT_EQ(RCL_RET_OK, rcl_shutdown(&context)) << rcl_get_error_string().str;
ASSERT_EQ(RCL_RET_OK, rcl_context_fini(&context)) << rcl_get_error_string().str;
});
const rosidl_message_type_support_t * pn = const rosidl_message_type_support_t * pn =
ROSIDL_GET_MSG_TYPE_SUPPORT(lifecycle_msgs, msg, TransitionEvent); ROSIDL_GET_MSG_TYPE_SUPPORT(lifecycle_msgs, msg, TransitionEvent);
const rosidl_service_type_support_t * cs = const rosidl_service_type_support_t * cs =
@ -326,6 +336,7 @@ TEST(TestRclLifecycle, state_machine) {
// Node is null // Node is null
ret = rcl_lifecycle_state_machine_fini(&state_machine, nullptr, &allocator); ret = rcl_lifecycle_state_machine_fini(&state_machine, nullptr, &allocator);
EXPECT_EQ(ret, RCL_RET_ERROR); EXPECT_EQ(ret, RCL_RET_ERROR);
rcutils_reset_error();
} }
TEST(TestRclLifecycle, state_transitions) { TEST(TestRclLifecycle, state_transitions) {
@ -348,15 +359,16 @@ TEST(TestRclLifecycle, state_transitions) {
ret = rcl_init(0, nullptr, &init_options, &context); ret = rcl_init(0, nullptr, &init_options, &context);
EXPECT_EQ(ret, RCL_RET_OK) << rcl_get_error_string().str; EXPECT_EQ(ret, RCL_RET_OK) << rcl_get_error_string().str;
OSRF_TESTING_TOOLS_CPP_SCOPE_EXIT(
{
ASSERT_EQ(RCL_RET_OK, rcl_shutdown(&context));
ASSERT_EQ(RCL_RET_OK, rcl_context_fini(&context));
});
ret = rcl_node_init(&node, "node", "namespace", &context, &options); ret = rcl_node_init(&node, "node", "namespace", &context, &options);
EXPECT_EQ(ret, RCL_RET_OK) << rcl_get_error_string().str; EXPECT_EQ(ret, RCL_RET_OK) << rcl_get_error_string().str;
OSRF_TESTING_TOOLS_CPP_SCOPE_EXIT(
{
ASSERT_EQ(RCL_RET_OK, rcl_node_fini(&node)) << rcl_get_error_string().str;
ASSERT_EQ(RCL_RET_OK, rcl_shutdown(&context)) << rcl_get_error_string().str;
ASSERT_EQ(RCL_RET_OK, rcl_context_fini(&context)) << rcl_get_error_string().str;
});
const rosidl_message_type_support_t * pn = const rosidl_message_type_support_t * pn =
ROSIDL_GET_MSG_TYPE_SUPPORT(lifecycle_msgs, msg, TransitionEvent); ROSIDL_GET_MSG_TYPE_SUPPORT(lifecycle_msgs, msg, TransitionEvent);
const rosidl_service_type_support_t * cs = const rosidl_service_type_support_t * cs =