introduce rclc_get_zero_initialized_lifecycle_node().#443
introduce rclc_get_zero_initialized_lifecycle_node().#443fujitatomoya wants to merge 1 commit intorollingfrom
Conversation
Signed-off-by: Tomoya Fujita <Tomoya.Fujita@sony.com>
There was a problem hiding this comment.
Pull request overview
This PR introduces a public helper API to obtain a zero-initialized rclc_lifecycle_node_t, and updates examples/tests/docs to use it to avoid crashes when stack memory is not implicitly zeroed (as described in #441).
Changes:
- Add
rclc_get_zero_initialized_lifecycle_node()to provide a safe zero-initialized lifecycle node struct. - Update lifecycle tests and the example lifecycle node to use the new initializer.
- Update the
rclc_lifecycleREADME to show the new initialization pattern.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| rclc_lifecycle/test/test_lifecycle.cpp | Switches stack lifecycle node variables to use the new zero-initializer. |
| rclc_lifecycle/src/rclc_lifecycle/rclc_lifecycle.c | Adds the rclc_get_zero_initialized_lifecycle_node() implementation. |
| rclc_lifecycle/include/rclc_lifecycle/rclc_lifecycle.h | Exposes the new API and documents it. |
| rclc_lifecycle/README.md | Updates the lifecycle node creation snippet to use the new initializer. |
| rclc_examples/src/example_lifecycle_node.c | Updates the example to use the new initializer. |
Comments suppressed due to low confidence (1)
rclc_lifecycle/README.md:39
- The README example calls
rclc_make_node_a_lifecycle_node()with fewer parameters than the current API requires (the function now takes a clock pointer and anenable_communication_interfaceflag). Since this PR touches this example block, it should be updated to match the actual function signature so users can copy/paste it successfully.
rclc_lifecycle_node_t lifecycle_node = rclc_get_zero_initialized_lifecycle_node();
rcl_ret_t rc = rclc_make_node_a_lifecycle_node(
&lifecycle_node,
&my_node,
&state_machine_,
&allocator);
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| static rclc_lifecycle_node_t null_lifecycle_node; | ||
| // Setting all fields to 0 | ||
| null_lifecycle_node.node = NULL; | ||
| null_lifecycle_node.state_machine = NULL; | ||
| memset(&null_lifecycle_node.callbacks, 0, sizeof(rclc_lifecycle_callback_map_t)); | ||
| null_lifecycle_node.publish_transitions = false; | ||
| memset(&null_lifecycle_node.cs_req, 0, sizeof(lifecycle_msgs__srv__ChangeState_Request)); | ||
| memset(&null_lifecycle_node.cs_res, 0, sizeof(lifecycle_msgs__srv__ChangeState_Response)); | ||
| memset(&null_lifecycle_node.gs_req, 0, sizeof(lifecycle_msgs__srv__GetState_Request)); | ||
| memset(&null_lifecycle_node.gs_res, 0, sizeof(lifecycle_msgs__srv__GetState_Response)); | ||
| memset(&null_lifecycle_node.gas_req, 0, sizeof(lifecycle_msgs__srv__GetAvailableStates_Request)); | ||
| memset(&null_lifecycle_node.gas_res, 0, sizeof(lifecycle_msgs__srv__GetAvailableStates_Response)); |
There was a problem hiding this comment.
rclc_get_zero_initialized_lifecycle_node() uses a writable static struct and mutates it on every call. This introduces a potential data race/UB if multiple threads call this function concurrently, and it’s also easy to miss new fields when the struct changes. Prefer returning a local zero-initialized value (e.g., via a compound literal or {0} init) so every field is covered and the function is thread-safe without shared state.
| static rclc_lifecycle_node_t null_lifecycle_node; | |
| // Setting all fields to 0 | |
| null_lifecycle_node.node = NULL; | |
| null_lifecycle_node.state_machine = NULL; | |
| memset(&null_lifecycle_node.callbacks, 0, sizeof(rclc_lifecycle_callback_map_t)); | |
| null_lifecycle_node.publish_transitions = false; | |
| memset(&null_lifecycle_node.cs_req, 0, sizeof(lifecycle_msgs__srv__ChangeState_Request)); | |
| memset(&null_lifecycle_node.cs_res, 0, sizeof(lifecycle_msgs__srv__ChangeState_Response)); | |
| memset(&null_lifecycle_node.gs_req, 0, sizeof(lifecycle_msgs__srv__GetState_Request)); | |
| memset(&null_lifecycle_node.gs_res, 0, sizeof(lifecycle_msgs__srv__GetState_Response)); | |
| memset(&null_lifecycle_node.gas_req, 0, sizeof(lifecycle_msgs__srv__GetAvailableStates_Request)); | |
| memset(&null_lifecycle_node.gas_res, 0, sizeof(lifecycle_msgs__srv__GetAvailableStates_Response)); | |
| rclc_lifecycle_node_t null_lifecycle_node = {0}; |
|
Pulls: #443
|
1 participant
Description
should closes #441
Is this user-facing behavior change?
No, but it is suggested to use new function to avoid the possible issues.
Did you use Generative AI?
Yes, Copilot Claude Sonnet 4.5
Additional Information
No