Skip to content

Commit

Permalink
Fix incorrect allocation size + minro refactor
Browse files Browse the repository at this point in the history
  • Loading branch information
jacobperron committed Nov 16, 2018
1 parent 1140112 commit 7aadb55
Show file tree
Hide file tree
Showing 2 changed files with 16 additions and 20 deletions.
24 changes: 13 additions & 11 deletions rcl_action/src/rcl_action/action_server.c
Original file line number Diff line number Diff line change
Expand Up @@ -186,7 +186,7 @@ rcl_action_server_init(

// Initialize goal handle array
action_server->impl->goal_handles = NULL;
action_server->impl->num_goal_handles = 0;
action_server->impl->num_goal_handles = 0u;

return ret;
fail:
Expand Down Expand Up @@ -342,7 +342,7 @@ rcl_action_accept_new_goal(
const size_t num_goal_handles = action_server->impl->num_goal_handles;
// TODO(jacobperron): Don't allocate for every accepted goal handle,
// instead double the memory allocated if needed.
const size_t new_num_goal_handles = num_goal_handles + 1;
const size_t new_num_goal_handles = num_goal_handles + 1u;
void * tmp_ptr = allocator.reallocate(
goal_handles, new_num_goal_handles * sizeof(rcl_action_goal_handle_t *), allocator.state);
if (!tmp_ptr) {
Expand Down Expand Up @@ -412,7 +412,7 @@ rcl_action_get_goal_status_array(

// If number of goals is zero, then we don't need to do any allocation
size_t num_goals = action_server->impl->num_goal_handles;
if (0 == num_goals) {
if (0u == num_goals) {
status_message->msg.status_list.size = 0u;
return RCL_RET_OK;
}
Expand All @@ -428,7 +428,7 @@ rcl_action_get_goal_status_array(
}

// Populate status array
for (size_t i = 0; i < num_goals; ++i) {
for (size_t i = 0u; i < num_goals; ++i) {
ret = rcl_action_goal_handle_get_info(
action_server->impl->goal_handles[i], &status_message->msg.status_list.data[i].goal_info);
if (RCL_RET_OK != ret) {
Expand Down Expand Up @@ -510,7 +510,7 @@ rcl_action_expire_goals(
rcl_action_goal_info_t goal_info;
int64_t goal_time;
size_t num_goal_handles = action_server->impl->num_goal_handles;
for (size_t i = 0; i < num_goal_handles; ++i) {
for (size_t i = 0u; i < num_goal_handles; ++i) {
goal_handle = action_server->impl->goal_handles[i];
// Expiration only applys to terminated goals
if (rcl_action_goal_handle_is_active(goal_handle)) {
Expand All @@ -534,11 +534,13 @@ rcl_action_expire_goals(

// Shrink goal handle array if some goals expired
if (num_goals_expired > 0u) {
if (0 == num_goal_handles) {
if (0u == num_goal_handles) {
allocator.deallocate(action_server->impl->goal_handles, allocator.state);
} else {
void * tmp_ptr = allocator.reallocate(
action_server->impl->goal_handles, num_goal_handles, allocator.state);
action_server->impl->goal_handles,
num_goal_handles * sizeof(rcl_action_goal_handle_t *),
allocator.state);
if (!tmp_ptr) {
RCL_SET_ERROR_MSG("failed to shrink size of goal handle array");
ret_final = RCL_RET_ERROR;
Expand Down Expand Up @@ -601,7 +603,7 @@ rcl_action_process_cancel_request(
// UUID is not zero and timestamp is zero; cancel exactly one goal (if it exists)
rcl_action_goal_info_t goal_info = rcl_action_get_zero_initialized_goal_info();
rcl_action_goal_handle_t * goal_handle;
for (size_t i = 0; i < total_num_goals; ++i) {
for (size_t i = 0u; i < total_num_goals; ++i) {
goal_handle = action_server->impl->goal_handles[i];
rcl_ret_t ret = rcl_action_goal_handle_get_info(goal_handle, &goal_info);
if (RCL_RET_OK != ret) {
Expand All @@ -627,7 +629,7 @@ rcl_action_process_cancel_request(
// Also cancel any goal matching the UUID in the cancel request
rcl_action_goal_info_t goal_info = rcl_action_get_zero_initialized_goal_info();
rcl_action_goal_handle_t * goal_handle;
for (size_t i = 0; i < total_num_goals; ++i) {
for (size_t i = 0u; i < total_num_goals; ++i) {
goal_handle = action_server->impl->goal_handles[i];
rcl_ret_t ret = rcl_action_goal_handle_get_info(goal_handle, &goal_info);
if (RCL_RET_OK != ret) {
Expand Down Expand Up @@ -663,7 +665,7 @@ rcl_action_process_cancel_request(

// Transition goals to canceling and add to response
rcl_action_goal_handle_t * goal_handle;
for (size_t i = 0; i < num_goals_to_cancel; ++i) {
for (size_t i = 0u; i < num_goals_to_cancel; ++i) {
goal_handle = goal_handles_to_cancel[i];
ret = rcl_action_update_goal_state(goal_handle, GOAL_EVENT_CANCEL);
if (RCL_RET_OK == ret) {
Expand Down Expand Up @@ -733,7 +735,7 @@ rcl_action_server_goal_exists(

rcl_action_goal_info_t gh_goal_info = rcl_action_get_zero_initialized_goal_info();
rcl_ret_t ret;
for (size_t i = 0; i < action_server->impl->num_goal_handles; ++i) {
for (size_t i = 0u; i < action_server->impl->num_goal_handles; ++i) {
ret = rcl_action_goal_handle_get_info(action_server->impl->goal_handles[i], &gh_goal_info);
if (RCL_RET_OK != ret) {
RCL_SET_ERROR_MSG("failed to get info for goal handle");
Expand Down
12 changes: 3 additions & 9 deletions rcl_action/test/rcl_action/test_action_server.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -478,19 +478,15 @@ TEST_F(TestActionServerCancelPolicy, test_action_process_cancel_request_single_g
{
// Request to cancel a specific goal
rcl_action_cancel_request_t cancel_request = rcl_action_get_zero_initialized_cancel_request();
for (int i = 0; i < UUID_SIZE; ++i) {
cancel_request.goal_info.uuid[i] = static_cast<uint8_t>(i + 2);
}
init_test_uuid0(cancel_request.goal_info.uuid);
rcl_action_cancel_response_t cancel_response = rcl_action_get_zero_initialized_cancel_response();
rcl_ret_t ret = rcl_action_process_cancel_request(
&this->action_server, &cancel_request, &cancel_response);
EXPECT_EQ(ret, RCL_RET_OK) << rcl_get_error_string().str;
EXPECT_NE(cancel_response.msg.goals_canceling.data, nullptr);
ASSERT_EQ(cancel_response.msg.goals_canceling.size, 1u);
rcl_action_goal_info_t * goal_info = &cancel_response.msg.goals_canceling.data[0];
for (int i = 0; i < UUID_SIZE; ++i) {
EXPECT_EQ(goal_info->uuid[i], static_cast<uint8_t>(i + 2));
}
EXPECT_TRUE(uuidcmp(goal_info->uuid, cancel_request.goal_info.uuid));
}

TEST_F(TestActionServerCancelPolicy, test_action_process_cancel_request_by_time)
Expand Down Expand Up @@ -539,7 +535,5 @@ TEST_F(TestActionServerCancelPolicy, test_action_process_cancel_request_by_time_
}
}
goal_info_out = &cancel_response.msg.goals_canceling.data[num_goals_canceling - 1];
for (int i = 0; i < UUID_SIZE; ++i) {
EXPECT_EQ(goal_info_out->uuid[i], static_cast<uint8_t>(i + goal_index));
}
EXPECT_TRUE(uuidcmp(goal_info_out->uuid, cancel_request.goal_info.uuid));
}

0 comments on commit 7aadb55

Please sign in to comment.