Skip to content

Potential null ptr passed to strcmp in remap.c #878

@nnmm

Description

@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.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions