Make finishing zero initialized log levels structure safe

Signed-off-by: Ivan Santiago Paunovic <ivanpauno@ekumenlabs.com>
ros2 · Jul 8, 2020 · d795983 · d795983
1 parent c8dab5f
commit d795983
Show file tree

Hide file tree

Showing 2 changed files with 9 additions and 11 deletions.
diff --git a/rcl/src/rcl/arguments.c b/rcl/src/rcl/arguments.c
@@ -150,8 +150,7 @@ rcl_arguments_get_log_levels(
   const rcl_allocator_t * allocator = &arguments->impl->allocator;
   RCL_CHECK_ALLOCATOR_WITH_MSG(allocator, "invalid allocator", return RCL_RET_INVALID_ARGUMENT);
 
-  rcl_ret_t ret = rcl_log_levels_copy(&arguments->impl->log_levels, log_levels);
-  return RCL_RET_OK;
+  return rcl_log_levels_copy(&arguments->impl->log_levels, log_levels);
 }
 
 /// Parse an argument that may or may not be a log level rule.
@@ -945,14 +944,12 @@ rcl_arguments_fini(
       args->impl->num_remap_rules = 0;
     }
 
-    if (args->impl->log_levels.logger_settings) {
-      rcl_ret_t log_levels_ret = rcl_log_levels_fini(&args->impl->log_levels);
-      if (log_levels_ret != RCL_RET_OK) {
-        ret = log_levels_ret;
-        RCUTILS_LOG_ERROR_NAMED(
-          ROS_PACKAGE_NAME,
-          "Failed to finalize log levels while finalizing arguments. Continuing...");
-      }
+    rcl_ret_t log_levels_ret = rcl_log_levels_fini(&args->impl->log_levels);
+    if (log_levels_ret != RCL_RET_OK) {
+      ret = log_levels_ret;
+      RCUTILS_LOG_ERROR_NAMED(
+        ROS_PACKAGE_NAME,
+        "Failed to finalize log levels while finalizing arguments. Continuing...");
     }
 
     args->impl->allocator.deallocate(args->impl->unparsed_args, args->impl->allocator.state);

diff --git a/rcl/src/rcl/log_level.c b/rcl/src/rcl/log_level.c
@@ -103,8 +103,9 @@ rcl_log_levels_fini(rcl_log_levels_t * log_levels)
 {
   RCL_CHECK_ARGUMENT_FOR_NULL(log_levels, RCL_RET_INVALID_ARGUMENT);
   const rcl_allocator_t * allocator = &log_levels->allocator;
-  RCL_CHECK_ALLOCATOR_WITH_MSG(allocator, "invalid allocator", return RCL_RET_INVALID_ARGUMENT);
   if (log_levels->logger_settings) {
+    // check allocator here, so it's safe to finish a zero initialized rcl_log_levels_t
+    RCL_CHECK_ALLOCATOR_WITH_MSG(allocator, "invalid allocator", return RCL_RET_INVALID_ARGUMENT);
     for (size_t i = 0; i < log_levels->num_logger_settings; ++i) {
       allocator->deallocate((void *)log_levels->logger_settings[i].name, allocator->state);
     }