From cc1c73a5e6f14e474d8424d888896bcaf11bf82c Mon Sep 17 00:00:00 2001 From: Chen Lihui Date: Fri, 4 Sep 2020 11:37:16 +0800 Subject: [PATCH 1/2] Fix that not to deallocate event impl in some failure case Signed-off-by: Chen Lihui --- rcl/src/rcl/event.c | 62 +++++++++++++++++++++++++++++---------------- 1 file changed, 40 insertions(+), 22 deletions(-) diff --git a/rcl/src/rcl/event.c b/rcl/src/rcl/event.c index 6fc432345..c3cd3e4c4 100644 --- a/rcl/src/rcl/event.c +++ b/rcl/src/rcl/event.c @@ -54,16 +54,6 @@ rcl_publisher_event_init( RCL_CHECK_ARGUMENT_FOR_NULL(publisher, RCL_RET_INVALID_ARGUMENT); rcl_allocator_t * allocator = &publisher->impl->options.allocator; RCL_CHECK_ALLOCATOR_WITH_MSG(allocator, "invalid allocator", return RCL_RET_INVALID_ARGUMENT); - - // Allocate space for the implementation struct. - event->impl = (rcl_event_impl_t *) allocator->allocate( - sizeof(rcl_event_impl_t), allocator->state); - RCL_CHECK_FOR_NULL_WITH_MSG( - event->impl, "allocating memory failed", ret = RCL_RET_BAD_ALLOC; return ret); - - event->impl->rmw_handle = rmw_get_zero_initialized_event(); - event->impl->allocator = *allocator; - rmw_event_type_t rmw_event_type = RMW_EVENT_INVALID; switch (event_type) { case RCL_PUBLISHER_OFFERED_DEADLINE_MISSED: @@ -79,10 +69,29 @@ rcl_publisher_event_init( RCL_SET_ERROR_MSG("Event type for publisher not supported"); return RCL_RET_INVALID_ARGUMENT; } - return rmw_publisher_event_init( + + // Allocate space for the implementation struct. + event->impl = (rcl_event_impl_t *) allocator->allocate( + sizeof(rcl_event_impl_t), allocator->state); + RCL_CHECK_FOR_NULL_WITH_MSG( + event->impl, "allocating memory failed", ret = RCL_RET_BAD_ALLOC; return ret); + + event->impl->rmw_handle = rmw_get_zero_initialized_event(); + event->impl->allocator = *allocator; + + ret = rmw_publisher_event_init( &event->impl->rmw_handle, publisher->impl->rmw_handle, rmw_event_type); + if (ret != RMW_RET_OK) { + goto fail; + } + + return RCL_RET_OK; +fail: + allocator->deallocate(event->impl, allocator->state); + event->impl = NULL; + return ret; } rcl_ret_t @@ -97,16 +106,6 @@ rcl_subscription_event_init( RCL_CHECK_ARGUMENT_FOR_NULL(subscription, RCL_RET_INVALID_ARGUMENT); rcl_allocator_t * allocator = &subscription->impl->options.allocator; RCL_CHECK_ALLOCATOR_WITH_MSG(allocator, "invalid allocator", return RCL_RET_INVALID_ARGUMENT); - - // Allocate space for the implementation struct. - event->impl = (rcl_event_impl_t *) allocator->allocate( - sizeof(rcl_event_impl_t), allocator->state); - RCL_CHECK_FOR_NULL_WITH_MSG( - event->impl, "allocating memory failed", ret = RCL_RET_BAD_ALLOC; return ret); - - event->impl->rmw_handle = rmw_get_zero_initialized_event(); - event->impl->allocator = *allocator; - rmw_event_type_t rmw_event_type = RMW_EVENT_INVALID; switch (event_type) { case RCL_SUBSCRIPTION_REQUESTED_DEADLINE_MISSED: @@ -125,10 +124,29 @@ rcl_subscription_event_init( RCL_SET_ERROR_MSG("Event type for subscription not supported"); return RCL_RET_INVALID_ARGUMENT; } - return rmw_subscription_event_init( + + // Allocate space for the implementation struct. + event->impl = (rcl_event_impl_t *) allocator->allocate( + sizeof(rcl_event_impl_t), allocator->state); + RCL_CHECK_FOR_NULL_WITH_MSG( + event->impl, "allocating memory failed", ret = RCL_RET_BAD_ALLOC; return ret); + + event->impl->rmw_handle = rmw_get_zero_initialized_event(); + event->impl->allocator = *allocator; + + ret = rmw_subscription_event_init( &event->impl->rmw_handle, subscription->impl->rmw_handle, rmw_event_type); + if (ret != RMW_RET_OK) { + goto fail; + } + + return RCL_RET_OK; +fail: + allocator->deallocate(event->impl, allocator->state); + event->impl = NULL; + return ret; } rcl_ret_t From 7ea5ebf421cc1d24da60bb16cb41be5e35ae2a91 Mon Sep 17 00:00:00 2001 From: Chen Lihui Date: Tue, 8 Sep 2020 22:10:55 +0800 Subject: [PATCH 2/2] update based on suggestions Signed-off-by: Chen Lihui Co-authored-by: Chris Lalancette --- rcl/src/rcl/event.c | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/rcl/src/rcl/event.c b/rcl/src/rcl/event.c index c3cd3e4c4..c79c60ce6 100644 --- a/rcl/src/rcl/event.c +++ b/rcl/src/rcl/event.c @@ -48,7 +48,6 @@ rcl_publisher_event_init( const rcl_publisher_t * publisher, const rcl_publisher_event_type_t event_type) { - rcl_ret_t ret = RCL_RET_OK; RCL_CHECK_ARGUMENT_FOR_NULL(event, RCL_RET_EVENT_INVALID); // Check publisher and allocator first, so allocator can be used with errors. RCL_CHECK_ARGUMENT_FOR_NULL(publisher, RCL_RET_INVALID_ARGUMENT); @@ -74,12 +73,12 @@ rcl_publisher_event_init( event->impl = (rcl_event_impl_t *) allocator->allocate( sizeof(rcl_event_impl_t), allocator->state); RCL_CHECK_FOR_NULL_WITH_MSG( - event->impl, "allocating memory failed", ret = RCL_RET_BAD_ALLOC; return ret); + event->impl, "allocating memory failed", return RCL_RET_BAD_ALLOC); event->impl->rmw_handle = rmw_get_zero_initialized_event(); event->impl->allocator = *allocator; - ret = rmw_publisher_event_init( + rmw_ret_t ret = rmw_publisher_event_init( &event->impl->rmw_handle, publisher->impl->rmw_handle, rmw_event_type); @@ -91,7 +90,7 @@ rcl_publisher_event_init( fail: allocator->deallocate(event->impl, allocator->state); event->impl = NULL; - return ret; + return rcl_convert_rmw_ret_to_rcl_ret(ret); } rcl_ret_t @@ -100,7 +99,6 @@ rcl_subscription_event_init( const rcl_subscription_t * subscription, const rcl_subscription_event_type_t event_type) { - rcl_ret_t ret = RCL_RET_OK; RCL_CHECK_ARGUMENT_FOR_NULL(event, RCL_RET_EVENT_INVALID); // Check subscription and allocator first, so allocator can be used with errors. RCL_CHECK_ARGUMENT_FOR_NULL(subscription, RCL_RET_INVALID_ARGUMENT); @@ -129,12 +127,12 @@ rcl_subscription_event_init( event->impl = (rcl_event_impl_t *) allocator->allocate( sizeof(rcl_event_impl_t), allocator->state); RCL_CHECK_FOR_NULL_WITH_MSG( - event->impl, "allocating memory failed", ret = RCL_RET_BAD_ALLOC; return ret); + event->impl, "allocating memory failed", return RCL_RET_BAD_ALLOC); event->impl->rmw_handle = rmw_get_zero_initialized_event(); event->impl->allocator = *allocator; - ret = rmw_subscription_event_init( + rmw_ret_t ret = rmw_subscription_event_init( &event->impl->rmw_handle, subscription->impl->rmw_handle, rmw_event_type); @@ -146,7 +144,7 @@ rcl_subscription_event_init( fail: allocator->deallocate(event->impl, allocator->state); event->impl = NULL; - return ret; + return rcl_convert_rmw_ret_to_rcl_ret(ret); } rcl_ret_t