From f0b0e984930ac93f0e69d1dfd0db8038f6be4737 Mon Sep 17 00:00:00 2001 From: lobotuerk Date: Mon, 14 Sep 2020 11:50:14 -0300 Subject: [PATCH 1/6] first Signed-off-by: lobotuerk --- rmw_fastrtps_shared_cpp/src/rmw_take.cpp | 22 +++++++++++++--------- 1 file changed, 13 insertions(+), 9 deletions(-) diff --git a/rmw_fastrtps_shared_cpp/src/rmw_take.cpp b/rmw_fastrtps_shared_cpp/src/rmw_take.cpp index 35b7bdfa5d..36883bd68c 100644 --- a/rmw_fastrtps_shared_cpp/src/rmw_take.cpp +++ b/rmw_fastrtps_shared_cpp/src/rmw_take.cpp @@ -60,10 +60,10 @@ _take( (void) allocation; *taken = false; - if (subscription->implementation_identifier != identifier) { - RMW_SET_ERROR_MSG("publisher handle not from this implementation"); - return RMW_RET_ERROR; - } + RMW_CHECK_TYPE_IDENTIFIERS_MATCH( + subscription handle, + subscription->implementation_identifier, identifier, + return RMW_RET_INCORRECT_RMW_IMPLEMENTATION) CustomSubscriberInfo * info = static_cast(subscription->data); RCUTILS_CHECK_FOR_NULL_WITH_MSG(info, "custom subscriber info is null", return RMW_RET_ERROR); @@ -175,11 +175,15 @@ __rmw_take( bool * taken, rmw_subscription_allocation_t * allocation) { - RCUTILS_CHECK_FOR_NULL_WITH_MSG( - subscription, "subscription pointer is null", return RMW_RET_ERROR); - RCUTILS_CHECK_FOR_NULL_WITH_MSG( - ros_message, "ros_message pointer is null", return RMW_RET_ERROR); - RCUTILS_CHECK_FOR_NULL_WITH_MSG(taken, "boolean flag for taken is null", return RMW_RET_ERROR); + RMW_CHECK_FOR_NULL_WITH_MSG( + subscription, "subscription info is null", + return RMW_RET_INVALID_ARGUMENT); + RMW_CHECK_FOR_NULL_WITH_MSG( + ros_message, "ros message handle is null", + return RMW_RET_INVALID_ARGUMENT); + RMW_CHECK_FOR_NULL_WITH_MSG( + taken, "taken handle is null", + return RMW_RET_INVALID_ARGUMENT); return _take(identifier, subscription, ros_message, taken, nullptr, allocation); } From 9392aaecce7a336d9579f31eef24ecd53007661f Mon Sep 17 00:00:00 2001 From: lobotuerk Date: Tue, 15 Sep 2020 10:17:06 -0300 Subject: [PATCH 2/6] fixup! first Signed-off-by: lobotuerk --- rmw_fastrtps_shared_cpp/src/rmw_take.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rmw_fastrtps_shared_cpp/src/rmw_take.cpp b/rmw_fastrtps_shared_cpp/src/rmw_take.cpp index 36883bd68c..0cdc09da52 100644 --- a/rmw_fastrtps_shared_cpp/src/rmw_take.cpp +++ b/rmw_fastrtps_shared_cpp/src/rmw_take.cpp @@ -176,7 +176,7 @@ __rmw_take( rmw_subscription_allocation_t * allocation) { RMW_CHECK_FOR_NULL_WITH_MSG( - subscription, "subscription info is null", + subscription, "subscription handle is null", return RMW_RET_INVALID_ARGUMENT); RMW_CHECK_FOR_NULL_WITH_MSG( ros_message, "ros message handle is null", From 56bbb98ce0a103610930f5732eb23c85b732c7c3 Mon Sep 17 00:00:00 2001 From: lobotuerk Date: Tue, 15 Sep 2020 10:51:32 -0300 Subject: [PATCH 3/6] added take_with_info bad returns Signed-off-by: lobotuerk --- rmw_fastrtps_shared_cpp/src/rmw_take.cpp | 20 ++++++++++++-------- 1 file changed, 12 insertions(+), 8 deletions(-) diff --git a/rmw_fastrtps_shared_cpp/src/rmw_take.cpp b/rmw_fastrtps_shared_cpp/src/rmw_take.cpp index 0cdc09da52..4be8ec2dd5 100644 --- a/rmw_fastrtps_shared_cpp/src/rmw_take.cpp +++ b/rmw_fastrtps_shared_cpp/src/rmw_take.cpp @@ -232,14 +232,18 @@ __rmw_take_with_info( rmw_message_info_t * message_info, rmw_subscription_allocation_t * allocation) { - RCUTILS_CHECK_FOR_NULL_WITH_MSG( - subscription, "subscription pointer is null", return RMW_RET_ERROR); - RCUTILS_CHECK_FOR_NULL_WITH_MSG( - ros_message, "ros_message pointer is null", return RMW_RET_ERROR); - RCUTILS_CHECK_FOR_NULL_WITH_MSG(taken, "boolean flag for taken is null", return RMW_RET_ERROR); - RCUTILS_CHECK_FOR_NULL_WITH_MSG( - message_info, "message info pointer is null", return RMW_RET_ERROR); - + RMW_CHECK_FOR_NULL_WITH_MSG( + message_info, "message info is null", + return RMW_RET_INVALID_ARGUMENT); + RMW_CHECK_FOR_NULL_WITH_MSG( + taken, "taken handle is null", + return RMW_RET_INVALID_ARGUMENT); + RMW_CHECK_FOR_NULL_WITH_MSG( + ros_message, "ros message handle is null", + return RMW_RET_INVALID_ARGUMENT); + RMW_CHECK_FOR_NULL_WITH_MSG( + subscription, "subscription handle is null", + return RMW_RET_INVALID_ARGUMENT); return _take(identifier, subscription, ros_message, taken, message_info, allocation); } From 1ebcf3b4d72eedea390dec2de8b01b1367c11fa9 Mon Sep 17 00:00:00 2001 From: lobotuerk Date: Thu, 17 Sep 2020 11:19:18 -0300 Subject: [PATCH 4/6] added take_sequence support Signed-off-by: lobotuerk --- rmw_fastrtps_shared_cpp/src/rmw_take.cpp | 37 ++++++++++++++---------- 1 file changed, 22 insertions(+), 15 deletions(-) diff --git a/rmw_fastrtps_shared_cpp/src/rmw_take.cpp b/rmw_fastrtps_shared_cpp/src/rmw_take.cpp index 4be8ec2dd5..eae6826e84 100644 --- a/rmw_fastrtps_shared_cpp/src/rmw_take.cpp +++ b/rmw_fastrtps_shared_cpp/src/rmw_take.cpp @@ -102,10 +102,10 @@ _take_sequence( bool taken_flag = false; rmw_ret_t ret = RMW_RET_OK; - if (subscription->implementation_identifier != identifier) { - RMW_SET_ERROR_MSG("publisher handle not from this implementation"); - return RMW_RET_ERROR; - } + RMW_CHECK_TYPE_IDENTIFIERS_MATCH( + subscription handle, + subscription->implementation_identifier, identifier, + return RMW_RET_INCORRECT_RMW_IMPLEMENTATION); CustomSubscriberInfo * info = static_cast(subscription->data); RCUTILS_CHECK_FOR_NULL_WITH_MSG(info, "custom subscriber info is null", return RMW_RET_ERROR); @@ -198,24 +198,31 @@ __rmw_take_sequence( size_t * taken, rmw_subscription_allocation_t * allocation) { - RCUTILS_CHECK_FOR_NULL_WITH_MSG( - subscription, "subscription pointer is null", return RMW_RET_INVALID_ARGUMENT); - RCUTILS_CHECK_FOR_NULL_WITH_MSG( - message_sequence, "message_sequence pointer is null", return RMW_RET_INVALID_ARGUMENT); - RCUTILS_CHECK_FOR_NULL_WITH_MSG( - message_info_sequence, "message_info_sequence pointer is null", - return RMW_RET_INVALID_ARGUMENT); - RCUTILS_CHECK_FOR_NULL_WITH_MSG( - taken, "size_t flag for count is null", return RMW_RET_INVALID_ARGUMENT); + RMW_CHECK_ARGUMENT_FOR_NULL( + subscription, RMW_RET_INVALID_ARGUMENT); + + RMW_CHECK_ARGUMENT_FOR_NULL( + message_sequence, RMW_RET_INVALID_ARGUMENT); + + RMW_CHECK_ARGUMENT_FOR_NULL( + message_info_sequence, RMW_RET_INVALID_ARGUMENT); + + RMW_CHECK_ARGUMENT_FOR_NULL( + taken, RMW_RET_INVALID_ARGUMENT); + + if (count == 0u) { + RMW_SET_ERROR_MSG("count cant be 0"); + return RMW_RET_INVALID_ARGUMENT; + } if (count > message_sequence->capacity) { RMW_SET_ERROR_MSG("Insufficient capacity in message_sequence"); - return RMW_RET_ERROR; + return RMW_RET_INVALID_ARGUMENT; } if (count > message_info_sequence->capacity) { RMW_SET_ERROR_MSG("Insufficient capacity in message_info_sequence"); - return RMW_RET_ERROR; + return RMW_RET_INVALID_ARGUMENT; } return _take_sequence( From 95970f2aa9da79fb83fee4b14033a118df387730 Mon Sep 17 00:00:00 2001 From: lobotuerk Date: Fri, 18 Sep 2020 16:00:29 -0300 Subject: [PATCH 5/6] reviews Signed-off-by: lobotuerk --- rmw_fastrtps_shared_cpp/src/rmw_take.cpp | 41 ++++++++++++------------ 1 file changed, 20 insertions(+), 21 deletions(-) diff --git a/rmw_fastrtps_shared_cpp/src/rmw_take.cpp b/rmw_fastrtps_shared_cpp/src/rmw_take.cpp index eae6826e84..bf36092302 100644 --- a/rmw_fastrtps_shared_cpp/src/rmw_take.cpp +++ b/rmw_fastrtps_shared_cpp/src/rmw_take.cpp @@ -175,15 +175,14 @@ __rmw_take( bool * taken, rmw_subscription_allocation_t * allocation) { - RMW_CHECK_FOR_NULL_WITH_MSG( - subscription, "subscription handle is null", - return RMW_RET_INVALID_ARGUMENT); - RMW_CHECK_FOR_NULL_WITH_MSG( - ros_message, "ros message handle is null", - return RMW_RET_INVALID_ARGUMENT); - RMW_CHECK_FOR_NULL_WITH_MSG( - taken, "taken handle is null", - return RMW_RET_INVALID_ARGUMENT); + RMW_CHECK_ARGUMENT_FOR_NULL( + subscription, RMW_RET_INVALID_ARGUMENT); + + RMW_CHECK_ARGUMENT_FOR_NULL( + ros_message, RMW_RET_INVALID_ARGUMENT); + + RMW_CHECK_ARGUMENT_FOR_NULL( + taken, RMW_RET_INVALID_ARGUMENT); return _take(identifier, subscription, ros_message, taken, nullptr, allocation); } @@ -239,18 +238,18 @@ __rmw_take_with_info( rmw_message_info_t * message_info, rmw_subscription_allocation_t * allocation) { - RMW_CHECK_FOR_NULL_WITH_MSG( - message_info, "message info is null", - return RMW_RET_INVALID_ARGUMENT); - RMW_CHECK_FOR_NULL_WITH_MSG( - taken, "taken handle is null", - return RMW_RET_INVALID_ARGUMENT); - RMW_CHECK_FOR_NULL_WITH_MSG( - ros_message, "ros message handle is null", - return RMW_RET_INVALID_ARGUMENT); - RMW_CHECK_FOR_NULL_WITH_MSG( - subscription, "subscription handle is null", - return RMW_RET_INVALID_ARGUMENT); + RMW_CHECK_ARGUMENT_FOR_NULL( + message_info, RMW_RET_INVALID_ARGUMENT); + + RMW_CHECK_ARGUMENT_FOR_NULL( + taken, RMW_RET_INVALID_ARGUMENT); + + RMW_CHECK_ARGUMENT_FOR_NULL( + ros_message, RMW_RET_INVALID_ARGUMENT); + + RMW_CHECK_ARGUMENT_FOR_NULL( + subscription, RMW_RET_INVALID_ARGUMENT); + return _take(identifier, subscription, ros_message, taken, message_info, allocation); } From a639de743e47ff1ba32fb7aee344a0cc75e214ee Mon Sep 17 00:00:00 2001 From: lobotuerk Date: Fri, 18 Sep 2020 17:30:52 -0300 Subject: [PATCH 6/6] reviews Signed-off-by: lobotuerk --- rmw_fastrtps_shared_cpp/src/rmw_take.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/rmw_fastrtps_shared_cpp/src/rmw_take.cpp b/rmw_fastrtps_shared_cpp/src/rmw_take.cpp index bf36092302..ea813d6abe 100644 --- a/rmw_fastrtps_shared_cpp/src/rmw_take.cpp +++ b/rmw_fastrtps_shared_cpp/src/rmw_take.cpp @@ -209,8 +209,8 @@ __rmw_take_sequence( RMW_CHECK_ARGUMENT_FOR_NULL( taken, RMW_RET_INVALID_ARGUMENT); - if (count == 0u) { - RMW_SET_ERROR_MSG("count cant be 0"); + if (0u == count) { + RMW_SET_ERROR_MSG("count cannot be 0"); return RMW_RET_INVALID_ARGUMENT; }