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

Out-of-scope memory use and memory leak issue (#172)

Browse source 
* * out-of-scope memory use and memory leak issue

temp going out of scope leaks the storage it points to
resource leak from local_namespace_

Signed-off-by: Ethan Gao <ethan.gao@linux.intel.com>

* exclude the memory free when the namespace given is null

Signed-off-by: Ethan Gao <ethan.gao@linux.intel.com>
This commit is contained in:
Ethan Gao 2017-11-07 05:33:07 +08:00 • committed by William Woodall
parent 2558923315
commit f7d7df6d69
1 changed files with 28 additions and 1 deletions
Download patch file Download diff file Expand all files Collapse all files

29
rcl/src/rcl/node.c
View file

@ -146,11 +146,13 @@ rcl_node_init(
// Have this special case to avoid a memory allocation when "" is passed. // Have this special case to avoid a memory allocation when "" is passed.
local_namespace_ = "/"; local_namespace_ = "/";
} }
char * temp = NULL;
// If the namespace does not start with a /, add one. // If the namespace does not start with a /, add one.
if (namespace_length > 0 && namespace_[0] != '/') { if (namespace_length > 0 && namespace_[0] != '/') {
// TODO(wjwwood): replace with generic strcat that takes an allocator once available // TODO(wjwwood): replace with generic strcat that takes an allocator once available
// length + 2, because new leading / and terminating \0 // length + 2, because new leading / and terminating \0
char * temp = (char *)allocator->allocate(namespace_length + 2, allocator->state); temp = (char *)allocator->allocate(namespace_length + 2, allocator->state);
RCL_CHECK_FOR_NULL_WITH_MSG( RCL_CHECK_FOR_NULL_WITH_MSG(
temp, "allocating memory failed", return RCL_RET_BAD_ALLOC, *allocator); temp, "allocating memory failed", return RCL_RET_BAD_ALLOC, *allocator);
temp[0] = '/'; temp[0] = '/';
@ -163,6 +165,9 @@ rcl_node_init(
ret = rmw_validate_namespace(local_namespace_, &validation_result, NULL); ret = rmw_validate_namespace(local_namespace_, &validation_result, NULL);
if (ret != RMW_RET_OK) { if (ret != RMW_RET_OK) {
RCL_SET_ERROR_MSG(rmw_get_error_string_safe(), *allocator); RCL_SET_ERROR_MSG(rmw_get_error_string_safe(), *allocator);
if (should_free_local_namespace_) {
allocator->deallocate((char *)local_namespace_, allocator->state);
}
return ret; return ret;
} }
if (validation_result != RMW_NAMESPACE_VALID) { if (validation_result != RMW_NAMESPACE_VALID) {
@ -176,11 +181,18 @@ rcl_node_init(
} else { } else {
RCL_SET_ERROR_MSG(msg, *allocator); RCL_SET_ERROR_MSG(msg, *allocator);
} }
if (should_free_local_namespace_) {
allocator->deallocate((char *)local_namespace_, allocator->state);
}
return RCL_RET_NODE_INVALID_NAMESPACE; return RCL_RET_NODE_INVALID_NAMESPACE;
} }
// Allocate space for the implementation struct. // Allocate space for the implementation struct.
node->impl = (rcl_node_impl_t *)allocator->allocate(sizeof(rcl_node_impl_t), allocator->state); node->impl = (rcl_node_impl_t *)allocator->allocate(sizeof(rcl_node_impl_t), allocator->state);
if (!node->impl && should_free_local_namespace_) {
allocator->deallocate((char *)local_namespace_, allocator->state);
}
RCL_CHECK_FOR_NULL_WITH_MSG( RCL_CHECK_FOR_NULL_WITH_MSG(
node->impl, "allocating memory failed", return RCL_RET_BAD_ALLOC, *allocator); node->impl, "allocating memory failed", return RCL_RET_BAD_ALLOC, *allocator);
node->impl->rmw_node_handle = NULL; node->impl->rmw_node_handle = NULL;
@ -216,6 +228,9 @@ rcl_node_init(
RCL_SET_ERROR_MSG( RCL_SET_ERROR_MSG(
"Environment variable " RCUTILS_STRINGIFY(ROS_SECURITY_ENABLE_VAR_NAME) "Environment variable " RCUTILS_STRINGIFY(ROS_SECURITY_ENABLE_VAR_NAME)
" could not be read", rcl_get_default_allocator()); " could not be read", rcl_get_default_allocator());
if (should_free_local_namespace_) {
allocator->deallocate((char *)local_namespace_, allocator->state);
}
return RCL_RET_ERROR; return RCL_RET_ERROR;
} }
@ -225,6 +240,9 @@ rcl_node_init(
RCL_SET_ERROR_MSG( RCL_SET_ERROR_MSG(
"Environment variable " RCUTILS_STRINGIFY(ROS_SECURITY_STRATEGY_VAR_NAME) "Environment variable " RCUTILS_STRINGIFY(ROS_SECURITY_STRATEGY_VAR_NAME)
" could not be read", rcl_get_default_allocator()); " could not be read", rcl_get_default_allocator());
if (should_free_local_namespace_) {
allocator->deallocate((char *)local_namespace_, allocator->state);
}
return RCL_RET_ERROR; return RCL_RET_ERROR;
} }
@ -246,6 +264,9 @@ rcl_node_init(
"SECURITY ERROR: unable to find a folder matching the node name in the " "SECURITY ERROR: unable to find a folder matching the node name in the "
RCUTILS_STRINGIFY(ROS_SECURITY_ROOT_DIRECTORY_VAR_NAME) RCUTILS_STRINGIFY(ROS_SECURITY_ROOT_DIRECTORY_VAR_NAME)
" directory while the requested security strategy requires it", *allocator); " directory while the requested security strategy requires it", *allocator);
if (should_free_local_namespace_) {
allocator->deallocate((char *)local_namespace_, allocator->state);
}
return RCL_RET_ERROR; return RCL_RET_ERROR;
} }
} }
@ -258,6 +279,7 @@ rcl_node_init(
// free local_namespace_ if necessary // free local_namespace_ if necessary
if (should_free_local_namespace_) { if (should_free_local_namespace_) {
allocator->deallocate((char *)local_namespace_, allocator->state); allocator->deallocate((char *)local_namespace_, allocator->state);
should_free_local_namespace_ = false;
} }
// instance id // instance id
node->impl->rcl_instance_id = rcl_get_instance_id(); node->impl->rcl_instance_id = rcl_get_instance_id();
@ -310,6 +332,11 @@ fail:
allocator->deallocate(node->impl, allocator->state); allocator->deallocate(node->impl, allocator->state);
} }
*node = rcl_get_zero_initialized_node(); *node = rcl_get_zero_initialized_node();
if (should_free_local_namespace_) {
allocator->deallocate((char *)local_namespace_, allocator->state);
local_namespace_ = NULL;
}
return fail_ret; return fail_ret;
} }