Increase rcl_lifecycle test coverage and add more safety checks (#649)
* Increase test coverage and add more safety checks Signed-off-by: Stephen Brawner <brawner@gmail.com> * More coverage Signed-off-by: Stephen Brawner <brawner@gmail.com> * Concatenating error messages Signed-off-by: Stephen Brawner <stephenbrawner@verbsurgical.com> * PR Fixup Signed-off-by: Stephen Brawner <stephenbrawner@verbsurgical.com> * Forgot a Signed-off-by: Stephen Brawner <stephenbrawner@verbsurgical.com> * PR Fixup Signed-off-by: Stephen Brawner <stephenbrawner@verbsurgical.com> * Moving var to top Signed-off-by: Stephen Brawner <stephenbrawner@verbsurgical.com> Co-authored-by: Stephen Brawner <stephenbrawner@verbsurgical.com>
This commit is contained in:
brawner
GitHub
parent
30e0536a1d
commit
7146919c3f
7 changed files with 591 additions and 19 deletions
|
|
@ -650,13 +650,15 @@ _register_transitions(
|
|||
return ret;
|
||||
}
|
||||
|
||||
// default implementation as despicted on
|
||||
// default implementation as depicted on
|
||||
// design.ros2.org
|
||||
rcl_ret_t
|
||||
rcl_lifecycle_init_default_state_machine(
|
||||
rcl_lifecycle_state_machine_t * state_machine, const rcutils_allocator_t * allocator)
|
||||
{
|
||||
rcl_ret_t ret = RCL_RET_ERROR;
|
||||
// Used for concatenating error messages in the fail: block.
|
||||
const char * fail_error_message = "";
|
||||
|
||||
// ***************************
|
||||
// register all primary states
|
||||
|
|
@ -691,8 +693,26 @@ rcl_lifecycle_init_default_state_machine(
|
|||
return ret;
|
||||
|
||||
fail:
|
||||
// If rcl_lifecycle_transition_map_fini() fails, it will clobber the error string here.
|
||||
// Concatenate the error strings if that happens
|
||||
if (rcl_error_is_set()) {
|
||||
fail_error_message = rcl_get_error_string().str;
|
||||
}
|
||||
|
||||
if (rcl_lifecycle_transition_map_fini(&state_machine->transition_map, allocator) != RCL_RET_OK) {
|
||||
RCL_SET_ERROR_MSG("could not free lifecycle transition map. Leaking memory!\n");
|
||||
const char * fini_error = "";
|
||||
if (rcl_error_is_set()) {
|
||||
fini_error = rcl_get_error_string().str;
|
||||
rcl_reset_error();
|
||||
}
|
||||
RCL_SET_ERROR_MSG_WITH_FORMAT_STRING(
|
||||
"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",
|
||||
fail_error_message, fini_error);
|
||||
}
|
||||
|
||||
if (!rcl_error_is_set()) {
|
||||
RCL_SET_ERROR_MSG("Unspecified error in default_state_machine _register_transitions()");
|
||||
}
|
||||
return RCL_RET_ERROR;
|
||||
}
|
||||
|
|
|
|||
|
|
@ -40,6 +40,8 @@ rcl_lifecycle_get_zero_initialized_state()
|
|||
rcl_lifecycle_state_t state;
|
||||
state.id = 0;
|
||||
state.label = NULL;
|
||||
state.valid_transitions = NULL;
|
||||
state.valid_transition_size = 0;
|
||||
return state;
|
||||
}
|
||||
|
||||
|
|
@ -58,6 +60,10 @@ rcl_lifecycle_state_init(
|
|||
RCL_SET_ERROR_MSG("state pointer is null\n");
|
||||
return RCL_RET_ERROR;
|
||||
}
|
||||
if (!label) {
|
||||
RCL_SET_ERROR_MSG("State label is null\n");
|
||||
return RCL_RET_ERROR;
|
||||
}
|
||||
|
||||
state->id = id;
|
||||
state->label = rcutils_strndup(label, strlen(label), *allocator);
|
||||
|
|
@ -118,7 +124,22 @@ rcl_lifecycle_transition_init(
|
|||
|
||||
if (!transition) {
|
||||
RCL_SET_ERROR_MSG("transition pointer is null\n");
|
||||
return RCL_RET_OK;
|
||||
return RCL_RET_ERROR;
|
||||
}
|
||||
|
||||
if (!label) {
|
||||
RCL_SET_ERROR_MSG("label pointer is null\n");
|
||||
return RCL_RET_ERROR;
|
||||
}
|
||||
|
||||
if (!start) {
|
||||
RCL_SET_ERROR_MSG("start state pointer is null\n");
|
||||
return RCL_RET_ERROR;
|
||||
}
|
||||
|
||||
if (!goal) {
|
||||
RCL_SET_ERROR_MSG("goal state pointer is null\n");
|
||||
return RCL_RET_ERROR;
|
||||
}
|
||||
|
||||
transition->start = start;
|
||||
|
|
@ -140,7 +161,7 @@ rcl_lifecycle_transition_fini(
|
|||
const rcl_allocator_t * allocator)
|
||||
{
|
||||
if (!allocator) {
|
||||
RCL_SET_ERROR_MSG("can't initialize transition, no allocator given\n");
|
||||
RCL_SET_ERROR_MSG("can't finalize transition, no allocator given\n");
|
||||
return RCL_RET_ERROR;
|
||||
}
|
||||
// it is already NULL
|
||||
|
|
@ -192,6 +213,14 @@ rcl_lifecycle_state_machine_init(
|
|||
bool default_states,
|
||||
const rcl_allocator_t * allocator)
|
||||
{
|
||||
if (!state_machine) {
|
||||
RCL_SET_ERROR_MSG("State machine is null\n");
|
||||
return RCL_RET_ERROR;
|
||||
}
|
||||
if (!node_handle) {
|
||||
RCL_SET_ERROR_MSG("Node handle is null\n");
|
||||
return RCL_RET_ERROR;
|
||||
}
|
||||
if (!allocator) {
|
||||
RCL_SET_ERROR_MSG("can't initialize state machine, no allocator given\n");
|
||||
return RCL_RET_ERROR;
|
||||
|
|
@ -207,15 +236,12 @@ rcl_lifecycle_state_machine_init(
|
|||
}
|
||||
|
||||
if (default_states) {
|
||||
rcl_ret_t ret =
|
||||
rcl_lifecycle_init_default_state_machine(state_machine, allocator);
|
||||
ret = rcl_lifecycle_init_default_state_machine(state_machine, allocator);
|
||||
if (ret != RCL_RET_OK) {
|
||||
// init default state machine might have allocated memory,
|
||||
// so we have to call fini
|
||||
if (rcl_lifecycle_state_machine_fini(state_machine, node_handle, allocator) != RCL_RET_OK) {
|
||||
// error already set
|
||||
return RCL_RET_ERROR;
|
||||
}
|
||||
ret = rcl_lifecycle_state_machine_fini(state_machine, node_handle, allocator);
|
||||
return RCL_RET_ERROR;
|
||||
}
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -53,6 +53,11 @@ rcl_lifecycle_transition_map_fini(
|
|||
rcl_lifecycle_transition_map_t * transition_map,
|
||||
const rcutils_allocator_t * allocator)
|
||||
{
|
||||
if (!allocator) {
|
||||
RCL_SET_ERROR_MSG("can't free transition map, no allocator given\n");
|
||||
return RCL_RET_ERROR;
|
||||
}
|
||||
|
||||
rcl_ret_t fcn_ret = RCL_RET_OK;
|
||||
|
||||
// free valid transitions for all states
|
||||
|
|
@ -87,15 +92,16 @@ rcl_lifecycle_register_state(
|
|||
allocator, "invalid allocator", return RCUTILS_RET_INVALID_ARGUMENT)
|
||||
|
||||
// add new primary state memory
|
||||
transition_map->states_size += 1;
|
||||
unsigned int new_states_size = transition_map->states_size + 1;
|
||||
rcl_lifecycle_state_t * new_states = allocator->reallocate(
|
||||
transition_map->states,
|
||||
transition_map->states_size * sizeof(rcl_lifecycle_state_t),
|
||||
new_states_size * sizeof(rcl_lifecycle_state_t),
|
||||
allocator->state);
|
||||
if (!new_states) {
|
||||
RCL_SET_ERROR_MSG("failed to reallocate memory for new states");
|
||||
return RCL_RET_ERROR;
|
||||
}
|
||||
transition_map->states_size = new_states_size;
|
||||
transition_map->states = new_states;
|
||||
transition_map->states[transition_map->states_size - 1] = state;
|
||||
|
||||
|
|
@ -117,16 +123,22 @@ rcl_lifecycle_register_transition(
|
|||
return RCL_RET_ERROR;
|
||||
}
|
||||
|
||||
// we add a new transition, so increase the size
|
||||
transition_map->transitions_size += 1;
|
||||
rcl_lifecycle_state_t * goal = rcl_lifecycle_get_state(transition_map, transition.goal->id);
|
||||
if (!goal) {
|
||||
RCL_SET_ERROR_MSG_WITH_FORMAT_STRING("state %u is not registered\n", transition.goal->id);
|
||||
return RCL_RET_ERROR;
|
||||
}
|
||||
// Attempt to add new transition, don't update map if it fails
|
||||
unsigned int new_transitions_size = transition_map->transitions_size + 1;
|
||||
rcl_lifecycle_transition_t * new_transitions = allocator->reallocate(
|
||||
transition_map->transitions,
|
||||
transition_map->transitions_size * sizeof(rcl_lifecycle_transition_t),
|
||||
new_transitions_size * sizeof(rcl_lifecycle_transition_t),
|
||||
allocator->state);
|
||||
if (!new_transitions) {
|
||||
RCL_SET_ERROR_MSG("failed to reallocate memory for new transitions");
|
||||
return RCL_RET_BAD_ALLOC;
|
||||
}
|
||||
transition_map->transitions_size = new_transitions_size;
|
||||
transition_map->transitions = new_transitions;
|
||||
// finally set the new transition to the end of the array
|
||||
transition_map->transitions[transition_map->transitions_size - 1] = transition;
|
||||
|
|
@ -134,15 +146,16 @@ rcl_lifecycle_register_transition(
|
|||
// we have to copy the transitons here once more to the actual state
|
||||
// as we can't assign only the pointer. This pointer gets invalidated whenever
|
||||
// we add a new transition and re-shuffle/re-allocate new memory for it.
|
||||
state->valid_transition_size += 1;
|
||||
unsigned int new_valid_transitions_size = state->valid_transition_size + 1;
|
||||
rcl_lifecycle_transition_t * new_valid_transitions = allocator->reallocate(
|
||||
state->valid_transitions,
|
||||
state->valid_transition_size * sizeof(rcl_lifecycle_transition_t),
|
||||
new_valid_transitions_size * sizeof(rcl_lifecycle_transition_t),
|
||||
allocator->state);
|
||||
if (!new_valid_transitions) {
|
||||
RCL_SET_ERROR_MSG("failed to reallocate memory for new transitions on state");
|
||||
return RCL_RET_ERROR;
|
||||
}
|
||||
state->valid_transition_size = new_valid_transitions_size;
|
||||
state->valid_transitions = new_valid_transitions;
|
||||
|
||||
state->valid_transitions[state->valid_transition_size - 1] = transition;
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue