Potential null ptr passed to strcmp in remap.c

[nnmm]

Bug report

Required Info:

• Operating System: Ubuntu 20.04
• Clang-tidy version: 10.0.0
• Version or commit hash: 30464d1

Steps to reproduce issue

# Produce compilation database while building ROS 2 rolling
colcon build --symlink-install --merge-install --cmake-args -DCMAKE_EXPORT_COMPILE_COMMANDS=1
clang-tidy -p build/compile_commands.json src/ros2/rcl/rcl/src/rcl/*.c

Expected behavior

The arguments to strcmp are never null.

Actual behavior

~/ros2_rolling/src/ros2/rcl/rcl/src/rcl/remap.c:145:23: warning: Null pointer passed to 2nd parameter expecting 'nonnull' [clang-analyzer-core.NonNullParamChecker]
      matched = (0 == strcmp(expanded_match, name));
                      ^
~/ros2_rolling/src/ros2/rcl/rcl/src/rcl/remap.c:316:3: note: Assuming the condition is false
  RCUTILS_CAN_SET_MSG_AND_RETURN_WITH_ERROR_OF(RCL_RET_INVALID_ARGUMENT);
  ^

... many more notes to the effect that none of the early returns are taken ...

~/ros2_rolling/src/ros2/rcl/rcl/src/rcl/remap.c:324:61: note: Passing null pointer value via 4th parameter 'name'
    local_arguments, global_arguments, RCL_NAMESPACE_REMAP, NULL, node_name, NULL, NULL,
                                                            ^
/usr/lib/llvm-10/lib/clang/10.0.0/include/stddef.h:89:16: note: expanded from macro 'NULL'
#  define NULL ((void*)0)
               ^
~/ros2_rolling/src/ros2/rcl/rcl/src/rcl/remap.c:323:10: note: Calling 'rcl_remap_name'
  return rcl_remap_name(
         ^
...

~/ros2_rolling/src/ros2/rcl/rcl/src/rcl/remap.c:193:7: note: Passing null pointer value via 4th parameter 'name'
      name, node_name, node_namespace, substitutions, allocator, &rule);
      ^
~/ros2_rolling/src/ros2/rcl/rcl/src/rcl/remap.c:191:21: note: Calling 'rcl_remap_first_match'
    rcl_ret_t ret = rcl_remap_first_match(
                    ^

...

~/ros2_rolling/src/ros2/rcl/rcl/src/rcl/remap.c:145:23: note: Null pointer passed to 2nd parameter expecting 'nonnull'
      matched = (0 == strcmp(expanded_match, name));

Additional information

I was running clang-tidy on the codebase but I'm actually not familiar with that part of ROS, so I hope it's not a false positive. It also suggested using memset_s instead of memset.

[clalancette]

I've also seen this error when running clang static analysis over the codebase. I don't think this can happen in practice, but there is a theoretical codepath where it can happen. My local fix was to make this change:

      if (NULL != name) {
        matched = (0 == strcmp(expanded_match, name));
      }

If you'd like to submit a PR with that (or something similar), I'd be happy to review.

[fujitatomoya]

@nnmm
CC: @clalancette

i think we can close this.