Skip to content

Commit

Permalink
Add deallocate calls to free strdup allocated memory (#737)
Browse files Browse the repository at this point in the history
* Add deallocate calls to free strdup allocated memory
* Add variables to know if free is required
* Reformat not use extra booleans
* Address peer review comments

Signed-off-by: Jorge Perez <jjperez@ekumenlabs.com>
  • Loading branch information
Blast545 authored and ahcorde committed Nov 2, 2020
1 parent f3a8bb0 commit 72837b7
Showing 1 changed file with 20 additions and 9 deletions.
29 changes: 20 additions & 9 deletions rcl/src/rcl/rmw_implementation_identifier_check.c
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,7 @@ rcl_ret_t rcl_rmw_implementation_identifier_check(void)
// If the environment variable RMW_IMPLEMENTATION is set, or
// the environment variable RCL_ASSERT_RMW_ID_MATCHES is set,
// check that the result of `rmw_get_implementation_identifier` matches.
rcl_ret_t ret = RCL_RET_OK;
rcl_allocator_t allocator = rcl_get_default_allocator();
char * expected_rmw_impl = NULL;
const char * expected_rmw_impl_env = NULL;
Expand Down Expand Up @@ -89,14 +90,16 @@ rcl_ret_t rcl_rmw_implementation_identifier_check(void)
"Error getting env var '"
RCUTILS_STRINGIFY(RCL_ASSERT_RMW_ID_MATCHES_ENV_VAR_NAME) "': %s\n",
get_env_error_str);
return RCL_RET_ERROR;
ret = RCL_RET_ERROR;
goto cleanup;
}
if (strlen(asserted_rmw_impl_env) > 0) {
// Copy the environment variable so it doesn't get over-written by the next getenv call.
asserted_rmw_impl = rcutils_strdup(asserted_rmw_impl_env, allocator);
if (!asserted_rmw_impl) {
RCL_SET_ERROR_MSG("allocation failed");
return RCL_RET_BAD_ALLOC;
ret = RCL_RET_BAD_ALLOC;
goto cleanup;
}
}

Expand All @@ -107,20 +110,23 @@ rcl_ret_t rcl_rmw_implementation_identifier_check(void)
"variables do not match, exiting with %d.",
expected_rmw_impl, asserted_rmw_impl, RCL_RET_ERROR
);
return RCL_RET_ERROR;
ret = RCL_RET_ERROR;
goto cleanup;
}

// Collapse the expected_rmw_impl and asserted_rmw_impl variables so only expected_rmw_impl needs
// to be used from now on.
if (expected_rmw_impl && asserted_rmw_impl) {
// The strings at this point must be equal.
// No need for asserted_rmw_impl anymore, free the memory.
allocator.deallocate((char *)asserted_rmw_impl, allocator.state);
allocator.deallocate(asserted_rmw_impl, allocator.state);
asserted_rmw_impl = NULL;
} else {
// One or none are set.
// If asserted_rmw_impl has contents, move it over to expected_rmw_impl.
if (asserted_rmw_impl) {
expected_rmw_impl = asserted_rmw_impl;
asserted_rmw_impl = NULL;
}
}

Expand All @@ -137,7 +143,8 @@ rcl_ret_t rcl_rmw_implementation_identifier_check(void)
rmw_error_msg.str,
RCL_RET_ERROR
);
return RCL_RET_ERROR;
ret = RCL_RET_ERROR;
goto cleanup;
}
if (strcmp(actual_rmw_impl_id, expected_rmw_impl) != 0) {
RCL_SET_ERROR_MSG_WITH_FORMAT_STRING(
Expand All @@ -146,12 +153,16 @@ rcl_ret_t rcl_rmw_implementation_identifier_check(void)
actual_rmw_impl_id,
RCL_RET_MISMATCHED_RMW_ID
);
return RCL_RET_MISMATCHED_RMW_ID;
ret = RCL_RET_MISMATCHED_RMW_ID;
goto cleanup;
}
// Free the memory now that all checking has passed.
allocator.deallocate((char *)expected_rmw_impl, allocator.state);
}
return RCL_RET_OK;
ret = RCL_RET_OK;
// fallthrough
cleanup:
allocator.deallocate(expected_rmw_impl, allocator.state);
allocator.deallocate(asserted_rmw_impl, allocator.state);
return ret;
}

INITIALIZER(initialize) {
Expand Down

0 comments on commit 72837b7

Please sign in to comment.