diff --git a/rcl/CMakeLists.txt b/rcl/CMakeLists.txt index f0d54d5..a1658d5 100644 --- a/rcl/CMakeLists.txt +++ b/rcl/CMakeLists.txt @@ -13,6 +13,7 @@ include_directories(include) if(NOT WIN32) set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -Wall -Wextra") + set(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} -Wall -Wextra") endif() set(${PROJECT_NAME}_sources diff --git a/rcl/src/rcl/guard_condition.c b/rcl/src/rcl/guard_condition.c index 0d93155..b4d30b3 100644 --- a/rcl/src/rcl/guard_condition.c +++ b/rcl/src/rcl/guard_condition.c @@ -113,8 +113,7 @@ rcl_guard_condition_fini(rcl_guard_condition_t * guard_condition, rcl_node_t * n rcl_guard_condition_options_t rcl_guard_condition_get_default_options() { - static rcl_guard_condition_options_t default_options = {0}; - // Must set the allocator after because it is not a compile time constant. + static rcl_guard_condition_options_t default_options; default_options.allocator = rcl_get_default_allocator(); return default_options; } diff --git a/rcl/src/rcl/rcl.c b/rcl/src/rcl/rcl.c index 13ec6a8..6809407 100644 --- a/rcl/src/rcl/rcl.c +++ b/rcl/src/rcl/rcl.c @@ -26,7 +26,7 @@ extern "C" #include "rcl/error_handling.h" static atomic_bool __rcl_is_initialized = ATOMIC_VAR_INIT(false); -static rcl_allocator_t __rcl_allocator = {0}; +static rcl_allocator_t __rcl_allocator; static int __rcl_argc = 0; static char ** __rcl_argv = NULL; static atomic_uint_least64_t __rcl_instance_id = ATOMIC_VAR_INIT(0); @@ -36,7 +36,7 @@ static void __clean_up_init() { if (__rcl_argv) { - size_t i; + int i; for (i = 0; i < __rcl_argc; ++i) { if (__rcl_argv[i]) { // Use the old allocator. @@ -69,9 +69,14 @@ rcl_init(int argc, char ** argv, rcl_allocator_t allocator) RCL_SET_ERROR_MSG("rcl_init called while already initialized"); return RCL_RET_ALREADY_INIT; } + // There is a race condition between the time __rcl_is_initialized is set true, + // and when the allocator is set, in which rcl_shutdown() could get rcl_ok() as + // true and try to use the allocator, but it isn't set yet... + // A very unlikely race condition, but it is possile I think. + // I've documented that rcl_init() and rcl_shutdown() are not thread-safe with each other. + __rcl_allocator = allocator; // Set the new allocator. // TODO(wjwwood): Remove rcl specific command line arguments. // For now just copy the argc and argv. - __rcl_allocator = allocator; // Set the new allocator. __rcl_argc = argc; __rcl_argv = (char **)__rcl_allocator.allocate(sizeof(char *) * argc, __rcl_allocator.state); if (!__rcl_argv) { @@ -80,7 +85,7 @@ rcl_init(int argc, char ** argv, rcl_allocator_t allocator) goto fail; } memset(__rcl_argv, 0, sizeof(char **) * argc); - size_t i; + int i; for (i = 0; i < argc; ++i) { __rcl_argv[i] = (char *)__rcl_allocator.allocate(strlen(argv[i]), __rcl_allocator.state); memcpy(__rcl_argv[i], argv[i], strlen(argv[i])); diff --git a/rcl/src/rcl/subscription.c b/rcl/src/rcl/subscription.c index ce09153..2e58be0 100644 --- a/rcl/src/rcl/subscription.c +++ b/rcl/src/rcl/subscription.c @@ -137,7 +137,7 @@ rcl_take( rmw_message_info_t * message_info_local = message_info ? message_info : &dummy_message_info; // Call rmw_take_with_info. rmw_ret_t ret = - rmw_take_with_info(subscription->impl->rmw_handle, ros_message, taken, message_info); + rmw_take_with_info(subscription->impl->rmw_handle, ros_message, taken, message_info_local); if (ret != RMW_RET_OK) { RCL_SET_ERROR_MSG(rmw_get_error_string_safe()); return RCL_RET_ERROR; diff --git a/rcl/src/rcl/wait.c b/rcl/src/rcl/wait.c index 0323963..eb87cdc 100644 --- a/rcl/src/rcl/wait.c +++ b/rcl/src/rcl/wait.c @@ -42,7 +42,15 @@ typedef struct rcl_wait_set_impl_t rcl_wait_set_t rcl_get_zero_initialized_wait_set() { - static rcl_wait_set_t null_wait_set = {0}; + static rcl_wait_set_t null_wait_set = { + .subscriptions = NULL, + .size_of_subscriptions = 0, + .guard_conditions = NULL, + .size_of_guard_conditions = 0, + .timers = NULL, + .size_of_timers = 0, + .impl = NULL, + }; return null_wait_set; }