Skip to content

Commit

Permalink
Simplify local_namespace handling in rcl_node_init. (#1097)
Browse files Browse the repository at this point in the history
There is a comment in there about avoiding a memory allocation,
but that doesn't seem to make much sense to me; we are
going to be doing an allocation directly below, so avoiding
it here seems unnecessary and makes the code harder
to follow.  Simplify everything by always making an allocation

Signed-off-by: Chris Lalancette <[email protected]>
  • Loading branch information
clalancette authored Aug 28, 2023
1 parent 7fb6da3 commit 06d5745
Showing 1 changed file with 17 additions and 33 deletions.
50 changes: 17 additions & 33 deletions rcl/src/rcl/node.c
Original file line number Diff line number Diff line change
Expand Up @@ -12,11 +12,6 @@
// See the License for the specific language governing permissions and
// limitations under the License.

#ifdef __cplusplus
extern "C"
{
#endif

#include "rcl/node.h"

#include <limits.h>
Expand Down Expand Up @@ -162,24 +157,21 @@ rcl_node_init(
}

// Process the namespace.
size_t namespace_length = strlen(namespace_);
const char * local_namespace_ = namespace_;
bool should_free_local_namespace_ = false;
// If the namespace is just an empty string, replace with "/"
if (namespace_length == 0) {
// Have this special case to avoid a memory allocation when "" is passed.
local_namespace_ = "/";
}

// If the namespace does not start with a /, add one.
if (namespace_length > 0 && namespace_[0] != '/') {
const char * local_namespace_ = NULL;
if (namespace_[0] == '\0') {
// If the namespace is just an empty string, replace with "/"
local_namespace_ = rcutils_strdup("/", *allocator);
} else if (namespace_[0] == '/') {
local_namespace_ = rcutils_strdup(namespace_, *allocator);
} else {
// If the namespace does not start with a /, add one.
local_namespace_ = rcutils_format_string(*allocator, "/%s", namespace_);
RCL_CHECK_FOR_NULL_WITH_MSG(
local_namespace_,
"failed to format node namespace string",
ret = RCL_RET_BAD_ALLOC; goto cleanup);
should_free_local_namespace_ = true;
}
RCL_CHECK_FOR_NULL_WITH_MSG(
local_namespace_,
"failed to format node namespace string",
return RCL_RET_BAD_ALLOC);

// Make sure the node namespace is valid.
validation_result = 0;
ret = rmw_validate_namespace(local_namespace_, &validation_result, NULL);
Expand Down Expand Up @@ -233,10 +225,7 @@ rcl_node_init(
if (RCL_RET_OK != ret) {
goto fail;
} else if (NULL != remapped_namespace) {
if (should_free_local_namespace_) {
allocator->deallocate((char *)local_namespace_, allocator->state);
}
should_free_local_namespace_ = true;
allocator->deallocate((char *)local_namespace_, allocator->state);
local_namespace_ = remapped_namespace;
}

Expand Down Expand Up @@ -365,10 +354,9 @@ rcl_node_init(
ret = fail_ret;
// fall through from fail -> cleanup
cleanup:
if (should_free_local_namespace_) {
allocator->deallocate((char *)local_namespace_, allocator->state);
local_namespace_ = NULL;
}
allocator->deallocate((char *)local_namespace_, allocator->state);
local_namespace_ = NULL;

if (NULL != remapped_node_name) {
allocator->deallocate(remapped_node_name, allocator->state);
}
Expand Down Expand Up @@ -693,7 +681,3 @@ rcl_ret_t rcl_node_get_type_description_service(
*service_out = &node->impl->get_type_description_service;
return RCL_RET_OK;
}

#ifdef __cplusplus
}
#endif

0 comments on commit 06d5745

Please sign in to comment.