Repository navigation
Conversation
Signed-off-by: leoca <[email protected]>
fujitatomoya
left a comment
There was a problem hiding this comment.
@Dev-next-gen same here.
I could not run the full gtest suite here since I have no ROS 2 workspace on this machine
can you run the test to make sure it can be built and verified when submitting the PR? and please use our PR template that includes AI tool disclosure.
|
I moved the description onto the PR template and filled in the Generative AI section with the tool and model. The colcon build and test run you asked for is not done yet. The machine I work from has no ROS 2 installation, so the new test has still only been exercised through the standalone harness described in the PR. |
|
Thanks — you were right to ask, and it is done now. Official With the change, The array reports three entries after an allocation that never happened. The other four tests pass on both sides. Full output is in the updated description. The description now follows the ros2 template, with the Generative AI section filled in. |
fujitatomoya
left a comment
There was a problem hiding this comment.
lgtm with green CI.
|
Pulls: #593 |
|
@Mergifyio backport lyrical kilted jazzy humble |
✅ Backports have been createdDetails
|
(cherry picked from commit bf78574) Signed-off-by: leoca <[email protected]> Co-authored-by: Leo Camus <[email protected]>
(cherry picked from commit bf78574) Signed-off-by: leoca <[email protected]> Co-authored-by: Leo Camus <[email protected]>
(cherry picked from commit bf78574) Signed-off-by: leoca <[email protected]> Co-authored-by: Leo Camus <[email protected]>
(cherry picked from commit bf78574) Signed-off-by: leoca <[email protected]> Co-authored-by: Leo Camus <[email protected]>
Description
rcutils_string_array_init()writes the requested size into the array before it tries to allocate:When the allocation fails, the function returns
RCUTILS_RET_BAD_ALLOCbut leavessizeset to the requested count whiledatastaysNULL. 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 iterates0..size— walks a null pointer.This PR sets
sizeback to0on that error path, so a failed init leaves the array exactly asrcutils_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, andrcutils_string_array_fini()on it returnsRCUTILS_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 theinit_alloc_failure_leaves_array_emptytest intest/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-baseimage with the test dependencies (osrf_testing_tools_cpp,performance_test_fixture) installed byrosdep. Environment: Ubuntu 26.04.1, ROS 2 Rolling, gcc 15.2.0, cmake 4.2.3. Base:a4cba34onrolling.With this change applied,
colcon build --packages-select rcutilssucceeds and the test file passes:Then I reverted only
src/string_array.c, keeping the new test, rebuilt, and it fails on currentrolling:The array reports three entries after an allocation that never happened. The other four tests in the file pass on both sides.