From 3762d862df6e52ea173a167a9a40de4743267ea8 Mon Sep 17 00:00:00 2001 From: Tomoya Fujita Date: Sat, 21 Sep 2024 10:42:04 -0700 Subject: [PATCH] Fix NULL allocator and racy condition. (#1188) Signed-off-by: Tomoya Fujita --- rcl/src/rcl/arguments.c | 7 +++---- rcl/src/rcl/client.c | 5 +++-- rcl/src/rcl/context.c | 2 +- rcl/src/rcl/event.c | 3 ++- rcl/src/rcl/guard_condition.c | 8 +++----- rcl/src/rcl/lexer_lookahead.c | 5 ++--- rcl/src/rcl/node.c | 6 ++---- rcl/src/rcl/publisher.c | 5 +++-- rcl/src/rcl/remap.c | 5 ++--- rcl/src/rcl/service.c | 5 +++-- rcl/src/rcl/service_event_publisher.c | 3 ++- rcl/src/rcl/subscription.c | 5 +++-- rcl/src/rcl/timer.c | 3 ++- rcl/src/rcl/wait.c | 17 ++--------------- rcl_action/src/rcl_action/action_client.c | 5 +++-- rcl_action/src/rcl_action/action_server.c | 5 +++-- rcl_action/src/rcl_action/goal_handle.c | 3 ++- rcl_action/src/rcl_action/types.c | 12 ++++++++---- rcl_lifecycle/src/transition_map.c | 6 +----- 19 files changed, 50 insertions(+), 60 deletions(-) diff --git a/rcl/src/rcl/arguments.c b/rcl/src/rcl/arguments.c index f094ecd8a..fdbb4f9da 100644 --- a/rcl/src/rcl/arguments.c +++ b/rcl/src/rcl/arguments.c @@ -797,10 +797,9 @@ rcl_arguments_get_unparsed_ros( rcl_arguments_t rcl_get_zero_initialized_arguments(void) { - static rcl_arguments_t default_arguments = { - .impl = NULL - }; - return default_arguments; + // All members are initialized to 0 or NULL by C99 6.7.8/10. + static rcl_arguments_t zero_arguments; + return zero_arguments; } rcl_ret_t diff --git a/rcl/src/rcl/client.c b/rcl/src/rcl/client.c index 1b8088505..c7da5846b 100644 --- a/rcl/src/rcl/client.c +++ b/rcl/src/rcl/client.c @@ -55,7 +55,8 @@ struct rcl_client_impl_s rcl_client_t rcl_get_zero_initialized_client(void) { - static rcl_client_t null_client = {0}; + // All members are initialized to 0 or NULL by C99 6.7.8/10. + static rcl_client_t null_client; return null_client; } @@ -279,7 +280,7 @@ rcl_client_options_t rcl_client_get_default_options(void) { // !!! MAKE SURE THAT CHANGES TO THESE DEFAULTS ARE REFLECTED IN THE HEADER DOC STRING - static rcl_client_options_t default_options; + rcl_client_options_t default_options; // Must set the allocator and qos after because they are not a compile time constant. default_options.qos = rmw_qos_profile_services_default; default_options.allocator = rcl_get_default_allocator(); diff --git a/rcl/src/rcl/context.c b/rcl/src/rcl/context.c index 5414cc5e9..7f7d37144 100644 --- a/rcl/src/rcl/context.c +++ b/rcl/src/rcl/context.c @@ -28,7 +28,7 @@ extern "C" rcl_context_t rcl_get_zero_initialized_context(void) { - static rcl_context_t context = { + rcl_context_t context = { .impl = NULL, .instance_id_storage = {0}, }; diff --git a/rcl/src/rcl/event.c b/rcl/src/rcl/event.c index 9f1cf92dd..9cba507cc 100644 --- a/rcl/src/rcl/event.c +++ b/rcl/src/rcl/event.c @@ -38,7 +38,8 @@ extern "C" rcl_event_t rcl_get_zero_initialized_event(void) { - static rcl_event_t null_event = {0}; + // All members are initialized to 0 or NULL by C99 6.7.8/10. + static rcl_event_t null_event; return null_event; } diff --git a/rcl/src/rcl/guard_condition.c b/rcl/src/rcl/guard_condition.c index 3dd345ccc..957bbf996 100644 --- a/rcl/src/rcl/guard_condition.c +++ b/rcl/src/rcl/guard_condition.c @@ -36,10 +36,8 @@ struct rcl_guard_condition_impl_s rcl_guard_condition_t rcl_get_zero_initialized_guard_condition(void) { - static rcl_guard_condition_t null_guard_condition = { - .context = 0, - .impl = 0 - }; + // All members are initialized to 0 or NULL by C99 6.7.8/10. + static rcl_guard_condition_t null_guard_condition; return null_guard_condition; } @@ -144,7 +142,7 @@ rcl_guard_condition_options_t rcl_guard_condition_get_default_options(void) { // !!! MAKE SURE THAT CHANGES TO THESE DEFAULTS ARE REFLECTED IN THE HEADER DOC STRING - static rcl_guard_condition_options_t default_options; + rcl_guard_condition_options_t default_options; default_options.allocator = rcl_get_default_allocator(); return default_options; } diff --git a/rcl/src/rcl/lexer_lookahead.c b/rcl/src/rcl/lexer_lookahead.c index 62dc5ca1a..5c83a53a3 100644 --- a/rcl/src/rcl/lexer_lookahead.c +++ b/rcl/src/rcl/lexer_lookahead.c @@ -36,9 +36,8 @@ struct rcl_lexer_lookahead2_impl_s rcl_lexer_lookahead2_t rcl_get_zero_initialized_lexer_lookahead2(void) { - static rcl_lexer_lookahead2_t zero_initialized = { - .impl = NULL, - }; + // All members are initialized to 0 or NULL by C99 6.7.8/10. + static rcl_lexer_lookahead2_t zero_initialized; return zero_initialized; } diff --git a/rcl/src/rcl/node.c b/rcl/src/rcl/node.c index f6ec50c1e..1376f6660 100644 --- a/rcl/src/rcl/node.c +++ b/rcl/src/rcl/node.c @@ -97,10 +97,8 @@ const char * rcl_create_node_logger_name( rcl_node_t rcl_get_zero_initialized_node(void) { - static rcl_node_t null_node = { - .context = 0, - .impl = 0 - }; + // All members are initialized to 0 or NULL by C99 6.7.8/10. + static rcl_node_t null_node; return null_node; } diff --git a/rcl/src/rcl/publisher.c b/rcl/src/rcl/publisher.c index 60963254b..acd0df370 100644 --- a/rcl/src/rcl/publisher.c +++ b/rcl/src/rcl/publisher.c @@ -39,7 +39,8 @@ extern "C" rcl_publisher_t rcl_get_zero_initialized_publisher(void) { - static rcl_publisher_t null_publisher = {0}; + // All members are initialized to 0 or NULL by C99 6.7.8/10. + static rcl_publisher_t null_publisher; return null_publisher; } @@ -219,7 +220,7 @@ rcl_publisher_options_t rcl_publisher_get_default_options(void) { // !!! MAKE SURE THAT CHANGES TO THESE DEFAULTS ARE REFLECTED IN THE HEADER DOC STRING - static rcl_publisher_options_t default_options; + rcl_publisher_options_t default_options; // Must set the allocator and qos after because they are not a compile time constant. default_options.qos = rmw_qos_profile_default; default_options.allocator = rcl_get_default_allocator(); diff --git a/rcl/src/rcl/remap.c b/rcl/src/rcl/remap.c index d767ddd55..ccb96663e 100644 --- a/rcl/src/rcl/remap.c +++ b/rcl/src/rcl/remap.c @@ -31,9 +31,8 @@ extern "C" rcl_remap_t rcl_get_zero_initialized_remap(void) { - static rcl_remap_t default_rule = { - .impl = NULL - }; + // All members are initialized to 0 or NULL by C99 6.7.8/10. + static rcl_remap_t default_rule; return default_rule; } diff --git a/rcl/src/rcl/service.c b/rcl/src/rcl/service.c index 660fc8daf..4a2aec5ac 100644 --- a/rcl/src/rcl/service.c +++ b/rcl/src/rcl/service.c @@ -54,7 +54,8 @@ struct rcl_service_impl_s rcl_service_t rcl_get_zero_initialized_service(void) { - static rcl_service_t null_service = {0}; + // All members are initialized to 0 or NULL by C99 6.7.8/10. + static rcl_service_t null_service; return null_service; } @@ -285,7 +286,7 @@ rcl_service_options_t rcl_service_get_default_options(void) { // !!! MAKE SURE THAT CHANGES TO THESE DEFAULTS ARE REFLECTED IN THE HEADER DOC STRING - static rcl_service_options_t default_options; + rcl_service_options_t default_options; // Must set the allocator and qos after because they are not a compile time constant. default_options.qos = rmw_qos_profile_services_default; default_options.allocator = rcl_get_default_allocator(); diff --git a/rcl/src/rcl/service_event_publisher.c b/rcl/src/rcl/service_event_publisher.c index 777661ccd..00f0a5a36 100644 --- a/rcl/src/rcl/service_event_publisher.c +++ b/rcl/src/rcl/service_event_publisher.c @@ -31,7 +31,8 @@ rcl_service_event_publisher_t rcl_get_zero_initialized_service_event_publisher(void) { - static rcl_service_event_publisher_t zero_service_event_publisher = {0}; + // All members are initialized to 0 or NULL by C99 6.7.8/10. + static rcl_service_event_publisher_t zero_service_event_publisher; return zero_service_event_publisher; } diff --git a/rcl/src/rcl/subscription.c b/rcl/src/rcl/subscription.c index 8d4ea54aa..5eb6260cd 100644 --- a/rcl/src/rcl/subscription.c +++ b/rcl/src/rcl/subscription.c @@ -42,7 +42,8 @@ extern "C" rcl_subscription_t rcl_get_zero_initialized_subscription(void) { - static rcl_subscription_t null_subscription = {0}; + // All members are initialized to 0 or NULL by C99 6.7.8/10. + static rcl_subscription_t null_subscription; return null_subscription; } @@ -227,7 +228,7 @@ rcl_subscription_options_t rcl_subscription_get_default_options(void) { // !!! MAKE SURE THAT CHANGES TO THESE DEFAULTS ARE REFLECTED IN THE HEADER DOC STRING - static rcl_subscription_options_t default_options; + rcl_subscription_options_t default_options; // Must set these after declaration because they are not a compile time constants. default_options.qos = rmw_qos_profile_default; default_options.allocator = rcl_get_default_allocator(); diff --git a/rcl/src/rcl/timer.c b/rcl/src/rcl/timer.c index 879c148ed..efe728608 100644 --- a/rcl/src/rcl/timer.c +++ b/rcl/src/rcl/timer.c @@ -58,7 +58,8 @@ struct rcl_timer_impl_s rcl_timer_t rcl_get_zero_initialized_timer(void) { - static rcl_timer_t null_timer = {0}; + // All members are initialized to 0 or NULL by C99 6.7.8/10. + static rcl_timer_t null_timer; return null_timer; } diff --git a/rcl/src/rcl/wait.c b/rcl/src/rcl/wait.c index e6c4567c9..40d258710 100644 --- a/rcl/src/rcl/wait.c +++ b/rcl/src/rcl/wait.c @@ -63,21 +63,8 @@ struct rcl_wait_set_impl_s rcl_wait_set_t rcl_get_zero_initialized_wait_set(void) { - static rcl_wait_set_t null_wait_set = { - .subscriptions = NULL, - .size_of_subscriptions = 0, - .guard_conditions = NULL, - .size_of_guard_conditions = 0, - .clients = NULL, - .size_of_clients = 0, - .services = NULL, - .size_of_services = 0, - .timers = NULL, - .size_of_timers = 0, - .events = NULL, - .size_of_events = 0, - .impl = NULL, - }; + // All members are initialized to 0 or NULL by C99 6.7.8/10. + static rcl_wait_set_t null_wait_set; return null_wait_set; } diff --git a/rcl_action/src/rcl_action/action_client.c b/rcl_action/src/rcl_action/action_client.c index a173200cc..5e0ec1fab 100644 --- a/rcl_action/src/rcl_action/action_client.c +++ b/rcl_action/src/rcl_action/action_client.c @@ -43,7 +43,8 @@ extern "C" rcl_action_client_t rcl_action_get_zero_initialized_client(void) { - static rcl_action_client_t null_action_client = {0}; + // All members are initialized to 0 or NULL by C99 6.7.8/10. + static rcl_action_client_t null_action_client; return null_action_client; } @@ -269,7 +270,7 @@ rcl_action_client_fini(rcl_action_client_t * action_client, rcl_node_t * node) rcl_action_client_options_t rcl_action_client_get_default_options(void) { - static rcl_action_client_options_t default_options; + rcl_action_client_options_t default_options; default_options.goal_service_qos = rmw_qos_profile_services_default; default_options.cancel_service_qos = rmw_qos_profile_services_default; default_options.result_service_qos = rmw_qos_profile_services_default; diff --git a/rcl_action/src/rcl_action/action_server.c b/rcl_action/src/rcl_action/action_server.c index 47e662d7f..65957cdf6 100644 --- a/rcl_action/src/rcl_action/action_server.c +++ b/rcl_action/src/rcl_action/action_server.c @@ -44,7 +44,8 @@ rcl_action_goal_handle_set_goal_terminal_timestamp( rcl_action_server_t rcl_action_get_zero_initialized_server(void) { - static rcl_action_server_t null_action_server = {0}; + // All members are initialized to 0 or NULL by C99 6.7.8/10. + static rcl_action_server_t null_action_server; return null_action_server; } @@ -285,7 +286,7 @@ rcl_action_server_options_t rcl_action_server_get_default_options(void) { // !!! MAKE SURE THAT CHANGES TO THESE DEFAULTS ARE REFLECTED IN THE HEADER DOC STRING - static rcl_action_server_options_t default_options; + rcl_action_server_options_t default_options; default_options.goal_service_qos = rmw_qos_profile_services_default; default_options.cancel_service_qos = rmw_qos_profile_services_default; default_options.result_service_qos = rmw_qos_profile_services_default; diff --git a/rcl_action/src/rcl_action/goal_handle.c b/rcl_action/src/rcl_action/goal_handle.c index f326049d6..e7698d7d3 100644 --- a/rcl_action/src/rcl_action/goal_handle.c +++ b/rcl_action/src/rcl_action/goal_handle.c @@ -35,7 +35,8 @@ typedef struct rcl_action_goal_handle_impl_s rcl_action_goal_handle_t rcl_action_get_zero_initialized_goal_handle(void) { - static rcl_action_goal_handle_t null_handle = {0}; + // All members are initialized to 0 or NULL by C99 6.7.8/10. + static rcl_action_goal_handle_t null_handle; return null_handle; } diff --git a/rcl_action/src/rcl_action/types.c b/rcl_action/src/rcl_action/types.c index e6df16805..db640c3c6 100644 --- a/rcl_action/src/rcl_action/types.c +++ b/rcl_action/src/rcl_action/types.c @@ -24,28 +24,32 @@ extern "C" rcl_action_goal_info_t rcl_action_get_zero_initialized_goal_info(void) { - static rcl_action_goal_info_t goal_info = {{{0}}, {0, 0}}; + // All members are initialized to 0 or NULL by C99 6.7.8/10. + static rcl_action_goal_info_t goal_info; return goal_info; } rcl_action_goal_status_array_t rcl_action_get_zero_initialized_goal_status_array(void) { - static rcl_action_goal_status_array_t status_array = {{{0, 0, 0}}, {0, 0, 0, 0, 0}}; + // All members are initialized to 0 or NULL by C99 6.7.8/10. + static rcl_action_goal_status_array_t status_array; return status_array; } rcl_action_cancel_request_t rcl_action_get_zero_initialized_cancel_request(void) { - static rcl_action_cancel_request_t request = {{{{0}}, {0, 0}}}; + // All members are initialized to 0 or NULL by C99 6.7.8/10. + static rcl_action_cancel_request_t request; return request; } rcl_action_cancel_response_t rcl_action_get_zero_initialized_cancel_response(void) { - static rcl_action_cancel_response_t response = {{0, {0, 0, 0}}, {0, 0, 0, 0, 0}}; + // All members are initialized to 0 or NULL by C99 6.7.8/10. + static rcl_action_cancel_response_t response; return response; } diff --git a/rcl_lifecycle/src/transition_map.c b/rcl_lifecycle/src/transition_map.c index ce27f7fb5..e21c5c555 100644 --- a/rcl_lifecycle/src/transition_map.c +++ b/rcl_lifecycle/src/transition_map.c @@ -30,12 +30,8 @@ extern "C" rcl_lifecycle_transition_map_t rcl_lifecycle_get_zero_initialized_transition_map(void) { + // All members are initialized to 0 or NULL by C99 6.7.8/10. static rcl_lifecycle_transition_map_t transition_map; - transition_map.states = NULL; - transition_map.states_size = 0; - transition_map.transitions = NULL; - transition_map.transitions_size = 0; - return transition_map; }