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

rcl_action: address various clang static analysis fixes (#864) (#875)

Browse source 
* Address various clang static analysis fixes

Signed-off-by: Stephen Brawner <brawner@gmail.com>

* Add gtest assert

Signed-off-by: Stephen Brawner <brawner@gmail.com>

* Taking array by reference

Signed-off-by: Stephen Brawner <brawner@gmail.com>
This commit is contained in:
Stephen Brawner 2020-12-09 09:25:40 -08:00 • committed by GitHub
parent 814d958984
commit 6dbb394d2e
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
3 changed files with 18 additions and 4 deletions
Download patch file Download diff file Expand all files Collapse all files

9
rcl_action/src/rcl_action/action_server.c
View file

@ -656,13 +656,17 @@ rcl_action_expire_goals(
} }
} }
} }
ret_final = _recalculate_expire_timer( rcl_ret_t expire_timer_ret = _recalculate_expire_timer(
&action_server->impl->expire_timer, &action_server->impl->expire_timer,
action_server->impl->options.result_timeout.nanoseconds, action_server->impl->options.result_timeout.nanoseconds,
action_server->impl->goal_handles, action_server->impl->goal_handles,
action_server->impl->num_goal_handles, action_server->impl->num_goal_handles,
action_server->impl->clock); action_server->impl->clock);
if (RCL_RET_OK != expire_timer_ret) {
ret_final = expire_timer_ret;
}
// If argument is not null, then set it // If argument is not null, then set it
if (NULL != num_expired) { if (NULL != num_expired) {
(*num_expired) = num_goals_expired; (*num_expired) = num_goals_expired;
@ -795,8 +799,9 @@ rcl_action_process_cancel_request(
if (RCL_RET_OK != ret) { if (RCL_RET_OK != ret) {
if (RCL_RET_BAD_ALLOC == ret) { if (RCL_RET_BAD_ALLOC == ret) {
ret_final = RCL_RET_BAD_ALLOC; // error already set ret_final = RCL_RET_BAD_ALLOC; // error already set
} else {
ret_final = RCL_RET_ERROR; // error already set
} }
ret_final = RCL_RET_ERROR; // error already set
goto cleanup; goto cleanup;
} }

12
rcl_action/test/rcl_action/test_action_server.cpp
View file

@ -727,7 +727,11 @@ TEST_F(TestActionServer, test_action_server_get_goal_status_array)
EXPECT_NE(status_array.msg.status_list.data, nullptr); EXPECT_NE(status_array.msg.status_list.data, nullptr);
EXPECT_EQ(status_array.msg.status_list.size, 1u); EXPECT_EQ(status_array.msg.status_list.size, 1u);
rcl_action_goal_info_t * goal_info_out = &status_array.msg.status_list.data[0].goal_info; rcl_action_goal_info_t * goal_info_out = &status_array.msg.status_list.data[0].goal_info;
EXPECT_TRUE(uuidcmp(goal_info_out->goal_id.uuid, goal_info_in.goal_id.uuid));
// Passing this array by pointer can confuse scan-build into thinking the pointer is
// not checked for null. Passing by reference works fine
const auto & goal_info_out_uuid = goal_info_out->goal_id.uuid;
EXPECT_TRUE(uuidcmp(goal_info_out_uuid, goal_info_in.goal_id.uuid));
ret = rcl_action_goal_status_array_fini(&status_array); ret = rcl_action_goal_status_array_fini(&status_array);
ASSERT_EQ(ret, RCL_RET_OK); ASSERT_EQ(ret, RCL_RET_OK);
@ -870,7 +874,11 @@ TEST_F(TestActionServerCancelPolicy, test_action_process_cancel_request_single_g
ASSERT_EQ(cancel_response.msg.goals_canceling.size, 1u); ASSERT_EQ(cancel_response.msg.goals_canceling.size, 1u);
EXPECT_EQ(cancel_response.msg.return_code, action_msgs__srv__CancelGoal_Response__ERROR_NONE); EXPECT_EQ(cancel_response.msg.return_code, action_msgs__srv__CancelGoal_Response__ERROR_NONE);
rcl_action_goal_info_t * goal_info = &cancel_response.msg.goals_canceling.data[0]; rcl_action_goal_info_t * goal_info = &cancel_response.msg.goals_canceling.data[0];
EXPECT_TRUE(uuidcmp(goal_info->goal_id.uuid, cancel_request.goal_info.goal_id.uuid));
// Passing this array by pointer can confuse scan-build into thinking the pointer is
// not checked for null. Passing by reference works fine
const auto & goal_info_uuid = goal_info->goal_id.uuid;
EXPECT_TRUE(uuidcmp(goal_info_uuid, cancel_request.goal_info.goal_id.uuid));
EXPECT_EQ(RCL_RET_OK, rcl_action_cancel_response_fini(&cancel_response)); EXPECT_EQ(RCL_RET_OK, rcl_action_cancel_response_fini(&cancel_response));
} }
{ {

1
rcl_action/test/rcl_action/test_wait.cpp
View file

@ -796,6 +796,7 @@ TEST_F(TestActionServerWait, test_server_wait_set_get_entities_ready) {
EXPECT_EQ(ret, RCL_RET_INVALID_ARGUMENT); EXPECT_EQ(ret, RCL_RET_INVALID_ARGUMENT);
ret = rcl_wait_set_init(&wait_set, 0, 0, 1, 0, 3, 0, &this->context, rcl_get_default_allocator()); ret = rcl_wait_set_init(&wait_set, 0, 0, 1, 0, 3, 0, &this->context, rcl_get_default_allocator());
ASSERT_EQ(ret, RCL_RET_OK) << rcl_get_error_string().str;
wait_set.services[0] = &this->action_server.impl->goal_service; wait_set.services[0] = &this->action_server.impl->goal_service;
this->action_server.impl->wait_set_goal_service_index = 0; this->action_server.impl->wait_set_goal_service_index = 0;
wait_set.services[1] = &this->action_server.impl->cancel_service; wait_set.services[1] = &this->action_server.impl->cancel_service;