Skip to content

Reset string array size when allocation fails in init (backport #593) - #596

Merged
ahcorde merged 1 commit into
jazzyfrom
mergify/bp/jazzy/pr-593
Oct 5, 2026
Merged

ahcorde merged 1 commit into
jazzyfrom
mergify/bp/jazzy/pr-593

Conversation

@mergify

@mergify mergify Bot commented Sep 15, 2026

Copy link
Copy Markdown

Description

rcutils_string_array_init() writes the requested size into the array before it tries to allocate:

string_array->size = size;
string_array->data = allocator->zero_allocate(size, sizeof(char *), allocator->state);
if (NULL == string_array->data && 0 != size) {
  RCUTILS_SET_ERROR_MSG("failed to allocate string array");
  return RCUTILS_RET_BAD_ALLOC;
}

When the allocation fails, the function returns RCUTILS_RET_BAD_ALLOC but leaves size set to the requested count while data stays NULL. The array then advertises entries that cannot be read. A caller that checks the return value is fine, but one that inspects the array afterwards — or passes it to code that iterates 0..size — walks a null pointer.

This PR sets size back to 0 on that error path, so a failed init leaves the array exactly as rcutils_get_zero_initialized_string_array() would.

Fixes # (no issue filed; found while reading the allocation failure paths)

Is this user-facing behavior change?

Only on the failure path. On success nothing changes. After a failed rcutils_string_array_init() the array is now consistently empty (data == NULL, size == 0) instead of reporting a non-zero size with no backing storage, and rcutils_string_array_fini() on it returns RCUTILS_RET_OK.

Did you use Generative AI?

Yes. Claude Opus 5, through Claude Code, found the inconsistent error path, wrote the one-line change in src/string_array.c, wrote the init_alloc_failure_leaves_array_empty test in test/test_string_array.cpp, and drafted this description. The build and test verification below was run afterwards and is reported exactly as the container printed it.

Additional Information

Verification. My earlier note said I had no ROS 2 workspace on this machine. That is now done, in the official ros:rolling-ros-base image with the test dependencies (osrf_testing_tools_cpp, performance_test_fixture) installed by rosdep. Environment: Ubuntu 26.04.1, ROS 2 Rolling, gcc 15.2.0, cmake 4.2.3. Base: a4cba34 on rolling.

With this change applied, colcon build --packages-select rcutils succeeds and the test file passes:

build : OK
test_string_array.gtest.xml : total=5 failures=0 errors=0
  init_alloc_failure_leaves_array_empty        passed

Then I reverted only src/string_array.c, keeping the new test, rebuilt, and it fails on current rolling:

build : OK
test_string_array.gtest.xml : total=5 failures=1 errors=0
  init_alloc_failure_leaves_array_empty        FAILED
    test_string_array.cpp:84
    Expected equality of these values:
      0u          Which is: 0
      sa.size     Which is: 3

The array reports three entries after an allocation that never happened. The other four tests in the file pass on both sides.


This is an automatic backport of pull request #593 done by [Mergify](https://mergify.com).

@ahcorde

ahcorde commented Sep 15, 2026 •

Copy link
Copy Markdown
Contributor

Pulls: #596
Gist: https://gist.githubusercontent.com/ahcorde/5704993286ee0d138958bb5540e73db8/raw/d8fabe451bac968c720011a55073bacfd6223503/ros2.repos
BUILD args: --packages-above-and-dependencies rcutils
TEST args: --packages-above rcutils
ROS Distro: jazzy
Job: ci_launcher
ci_launcher ran: https://ci.ros2.org/job/ci_launcher/20453

  • Linux Build Status
  • Linux-aarch64 Build Status
  • Linux-rhel Build Status
  • Windows Build Status

@ahcorde
ahcorde merged commit 2641303 into jazzy Oct 5, 2026
2 checks passed
@ahcorde
ahcorde deleted the mergify/bp/jazzy/pr-593 branch October 5, 2026 08:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants