From 00fcf7066a3e634a541812928b510217173e64f2 Mon Sep 17 00:00:00 2001 From: Michael Carroll Date: Fri, 10 Apr 2020 15:08:26 -0500 Subject: [PATCH 1/8] Add API for taking a sequence of messages This will allow users to take multiple messages in one take call if they are available in the underlying middleware, verus having to make multiple rmw_take calls in a row. Signed-off-by: Michael Carroll --- rmw/CMakeLists.txt | 2 +- rmw/include/rmw/loaned_message_sequence.h | 44 -------- rmw/include/rmw/message_sequence.h | 57 +++++++++++ rmw/include/rmw/rmw.h | 29 ++++++ rmw/include/rmw/types.h | 8 -- rmw/src/loaned_message_sequence.c | 27 ----- rmw/src/message_sequence.c | 119 ++++++++++++++++++++++ rmw/test/CMakeLists.txt | 9 ++ rmw/test/test_message_sequence.cpp | 75 ++++++++++++++ 9 files changed, 290 insertions(+), 80 deletions(-) delete mode 100644 rmw/include/rmw/loaned_message_sequence.h create mode 100644 rmw/include/rmw/message_sequence.h delete mode 100644 rmw/src/loaned_message_sequence.c create mode 100644 rmw/src/message_sequence.c create mode 100644 rmw/test/test_message_sequence.cpp diff --git a/rmw/CMakeLists.txt b/rmw/CMakeLists.txt index 00eea6f4..2d914e10 100644 --- a/rmw/CMakeLists.txt +++ b/rmw/CMakeLists.txt @@ -29,7 +29,7 @@ set(rmw_sources "src/event.c" "src/init.c" "src/init_options.c" - "src/loaned_message_sequence.c" + "src/message_sequence.c" "src/names_and_types.c" "src/publisher_options.c" "src/sanity_checks.c" diff --git a/rmw/include/rmw/loaned_message_sequence.h b/rmw/include/rmw/loaned_message_sequence.h deleted file mode 100644 index 3d00d1e5..00000000 --- a/rmw/include/rmw/loaned_message_sequence.h +++ /dev/null @@ -1,44 +0,0 @@ -// Copyright 2019 Open Source Robotics Foundation, Inc. -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -#ifndef RMW__LOANED_MESSAGE_SEQUENCE_H_ -#define RMW__LOANED_MESSAGE_SEQUENCE_H_ - -#include - -#include "rmw/macros.h" -#include "rmw/visibility_control.h" - -#if __cplusplus -extern "C" -{ -#endif - -typedef struct RMW_PUBLIC_TYPE rmw_loaned_message_sequence_t -{ - void * message_sequence; - size_t size; - size_t capacity; -} rmw_loaned_message_sequence_t; - -RMW_PUBLIC -RMW_WARN_UNUSED -rmw_loaned_message_sequence_t -rmw_get_zero_initialized_loaned_message_sequence(void); - -#if __cplusplus -} -#endif - -#endif // RMW__LOANED_MESSAGE_SEQUENCE_H_ diff --git a/rmw/include/rmw/message_sequence.h b/rmw/include/rmw/message_sequence.h new file mode 100644 index 00000000..e8707593 --- /dev/null +++ b/rmw/include/rmw/message_sequence.h @@ -0,0 +1,57 @@ +// Copyright 2020 Open Source Robotics Foundation, Inc. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +#ifndef RMW__MESSAGE_SEQUENCE_H_ +#define RMW__MESSAGE_SEQUENCE_H_ + +#include + +#include "rmw/macros.h" +#include "rmw/visibility_control.h" +#include "rmw/types.h" + +#if __cplusplus +extern "C" +{ +#endif + +RMW_PUBLIC +rmw_message_sequence_t +rmw_get_zero_initialized_message_sequence(void); + +RMW_PUBLIC +rmw_ret_t +rmw_message_sequence_init(rmw_message_sequence_t * sequence, size_t size); + +RMW_PUBLIC +rmw_ret_t +rmw_message_sequence_fini(rmw_message_sequence_t * sequence); + +RMW_PUBLIC +rmw_message_info_sequence_t +rmw_get_zero_initialized_message_info_sequence(void); + +RMW_PUBLIC +rmw_ret_t +rmw_message_info_sequence_init(rmw_message_info_sequence_t * sequence, size_t size); + +RMW_PUBLIC +rmw_ret_t +rmw_message_info_sequence_fini(rmw_message_info_sequence_t * sequence); + +#if __cplusplus +} +#endif + +#endif // RMW__MESSAGE_SEQUENCE_H_ diff --git a/rmw/include/rmw/rmw.h b/rmw/include/rmw/rmw.h index 2a32ee49..d70e642d 100644 --- a/rmw/include/rmw/rmw.h +++ b/rmw/include/rmw/rmw.h @@ -98,6 +98,7 @@ extern "C" #include "rmw/macros.h" #include "rmw/qos_profiles.h" #include "rmw/subscription_options.h" +#include "rmw/message_sequence.h" #include "rmw/types.h" #include "rmw/visibility_control.h" @@ -716,6 +717,34 @@ rmw_take_with_info( rmw_message_info_t * message_info, rmw_subscription_allocation_t * allocation); +/// Take multiple incoming messages from a subscription with additional metadata. +/** + * Take a sequence of ROS messgages from a given subscription. + * + * While `count` messages may be requested, fewer messages may be available on the subscription. + * In this case, only the currently available messages will be returned. + * The `taken` flag indicate the number of messages actually taken. + * + * \param[in] subscription The subscription object to take from. + * \param[in] count Number of messages to attempt to take. + * \param[out] message_sequence The sequence ROS message data on success. + * \param[out] message_info_sequence The seqeucne of additional message metadata on success. + * \param[out] taken Number of messages actually taken from subscription. + * \param[in] allocation Preallocated buffer to use (may be NULL). + * \return `RMW_RET_OK` if successful, or + * \return `RMW_RET_ERROR` if an unexpected error occurs. + */ +RMW_PUBLIC +RMW_WARN_UNUSED +rmw_ret_t +rmw_take_sequence( + const rmw_subscription_t * subscription, + size_t count, + rmw_message_sequence_t * message_sequence, + rmw_message_info_sequence_t * message_info_sequence, + size_t * taken, + rmw_subscription_allocation_t * allocation); + /// Take a message without deserializing it. /** * The message is taken in its serialized form. In contrast to rmw_take, the message diff --git a/rmw/include/rmw/types.h b/rmw/include/rmw/types.h index 45914ac5..daf8da7e 100644 --- a/rmw/include/rmw/types.h +++ b/rmw/include/rmw/types.h @@ -29,7 +29,6 @@ extern "C" #include "rmw/init.h" #include "rmw/init_options.h" -#include "rmw/loaned_message_sequence.h" #include "rmw/ret_types.h" #include "rmw/security_options.h" #include "rmw/serialized_message.h" @@ -346,13 +345,6 @@ RMW_WARN_UNUSED rmw_message_info_t rmw_get_zero_initialized_message_info(void); -typedef struct RMW_PUBLIC_TYPE rmw_message_info_sequence_t -{ - rmw_message_info_t * message_info_sequence; - size_t size; - size_t capacity; -} rmw_message_info_sequence_t; - enum {RMW_QOS_POLICY_DEPTH_SYSTEM_DEFAULT = 0}; /// Type mapping of rcutils log severity types to rmw specific types. diff --git a/rmw/src/loaned_message_sequence.c b/rmw/src/loaned_message_sequence.c deleted file mode 100644 index 51ce5c35..00000000 --- a/rmw/src/loaned_message_sequence.c +++ /dev/null @@ -1,27 +0,0 @@ -// Copyright 2019 Open Source Robotics Foundation, Inc. -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -#include "rmw/loaned_message_sequence.h" - -rmw_loaned_message_sequence_t -rmw_get_zero_initialized_loaned_message_sequence(void) -{ - static rmw_loaned_message_sequence_t loaned_message_sequence = { - .message_sequence = NULL, - .size = 0u, - .capacity = 0u - }; - - return loaned_message_sequence; -} diff --git a/rmw/src/message_sequence.c b/rmw/src/message_sequence.c new file mode 100644 index 00000000..c208fa06 --- /dev/null +++ b/rmw/src/message_sequence.c @@ -0,0 +1,119 @@ +// Copyright 2020 Open Source Robotics Foundation, Inc. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +#include "rmw/message_sequence.h" + +rmw_message_sequence_t +rmw_get_zero_initialized_message_sequence(void) +{ + static rmw_message_sequence_t message_sequence = { + .data = NULL, + .size = 0u, + .capacity = 0u + }; + + return message_sequence; +} + +rmw_ret_t +rmw_message_sequence_init( + rmw_message_sequence_t * sequence, size_t size) +{ + RCUTILS_CHECK_ARGUMENT_FOR_NULL(sequence, RMW_RET_INVALID_ARGUMENT); + + void * data = NULL; + if (size) { + data = malloc(sizeof(void *) * size); + if (!data) { + return RMW_RET_BAD_ALLOC; + } + } + + sequence->data = data; + sequence->size = 0; + sequence->capacity = size; + + return RMW_RET_OK; +} + +rmw_ret_t +rmw_message_sequence_fini( + rmw_message_sequence_t * sequence) +{ + RCUTILS_CHECK_ARGUMENT_FOR_NULL(sequence, RMW_RET_INVALID_ARGUMENT); + + if (sequence->data) { + assert(sequence->capacity > 0); + free(sequence->data); + sequence->data = NULL; + sequence->size = 0; + sequence->capacity = 0; + } else { + assert(0 == sequence->size); + assert(0 == sequence->capacity); + } + + return RMW_RET_OK; +} + +rmw_message_info_sequence_t +rmw_get_zero_initialized_message_info_sequence(void) +{ + static rmw_message_info_sequence_t message_info_sequence = { + .data = NULL, + .size = 0u, + .capacity = 0u + }; + + return message_info_sequence; +} + +rmw_ret_t +rmw_message_info_sequence_init( + rmw_message_info_sequence_t * sequence, size_t size) +{ + RCUTILS_CHECK_ARGUMENT_FOR_NULL(sequence, RMW_RET_INVALID_ARGUMENT); + + rmw_message_info_t * data = NULL; + if (size) { + data = malloc(sizeof(rmw_message_info_t) * size); + if (!data) { + return RMW_RET_BAD_ALLOC; + } + } + sequence->data = data; + sequence->size = 0; + sequence->capacity = size; + return RMW_RET_OK; +} + +rmw_ret_t +rmw_message_info_sequence_fini( + rmw_message_info_sequence_t * sequence) +{ + RCUTILS_CHECK_ARGUMENT_FOR_NULL(sequence, RMW_RET_INVALID_ARGUMENT); + + if (sequence->data) { + assert(sequence->capacity > 0); + free(sequence->data); + sequence->data = NULL; + sequence->size = 0; + sequence->capacity = 0; + } else { + assert(0 == sequence->size); + assert(0 == sequence->capacity); + } + + return RMW_RET_OK; +} diff --git a/rmw/test/CMakeLists.txt b/rmw/test/CMakeLists.txt index cba7748e..b3a9425f 100644 --- a/rmw/test/CMakeLists.txt +++ b/rmw/test/CMakeLists.txt @@ -1,6 +1,15 @@ find_package(ament_cmake_gmock REQUIRED) find_package(osrf_testing_tools_cpp REQUIRED) +ament_add_gmock(test_message_sequence + test_message_sequence.cpp + # Append the directory of librmw so it is found at test time. + APPEND_LIBRARY_DIRS "$" +) +if(TARGET test_message_sequence) + target_link_libraries(test_message_sequence ${PROJECT_NAME}) +endif() + ament_add_gmock(test_serialized_message test_serialized_message.cpp # Append the directory of librmw so it is found at test time. diff --git a/rmw/test/test_message_sequence.cpp b/rmw/test/test_message_sequence.cpp new file mode 100644 index 00000000..292cd83a --- /dev/null +++ b/rmw/test/test_message_sequence.cpp @@ -0,0 +1,75 @@ +// Copyright 2018 Open Source Robotics Foundation, Inc. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +#include "gmock/gmock.h" + +#include "rcutils/allocator.h" + +#include "rmw/message_sequence.h" + +TEST(test_message_info_sequence, default_initialization) { + auto info_sequence = rmw_get_zero_initialized_message_info_sequence(); + + // Initialize the message sequence with a zero length + EXPECT_EQ(RMW_RET_OK, rmw_message_info_sequence_init(&info_sequence, 0)); + EXPECT_EQ(0u, info_sequence.size); + EXPECT_EQ(0u, info_sequence.capacity); + EXPECT_EQ(nullptr, info_sequence.data); + + EXPECT_EQ(RMW_RET_OK, rmw_message_info_sequence_fini(&info_sequence)); + EXPECT_EQ(0u, info_sequence.size); + EXPECT_EQ(0u, info_sequence.capacity); + EXPECT_EQ(nullptr, info_sequence.data); +} + +TEST(test_message_info_sequence, initialization_with_size) { + auto info_sequence = rmw_get_zero_initialized_message_info_sequence(); + + EXPECT_EQ(RMW_RET_OK, rmw_message_info_sequence_init(&info_sequence, 5)); + EXPECT_EQ(0u, info_sequence.size); + EXPECT_EQ(5u, info_sequence.capacity); + + EXPECT_EQ(RMW_RET_OK, rmw_message_info_sequence_fini(&info_sequence)); + EXPECT_EQ(0u, info_sequence.size); + EXPECT_EQ(0u, info_sequence.capacity); + EXPECT_EQ(nullptr, info_sequence.data); +} + +TEST(test_message_sequence, default_initialization) { + auto message_sequence = rmw_get_zero_initialized_message_sequence(); + + // Initialize the message sequence with a zero length + EXPECT_EQ(RMW_RET_OK, rmw_message_sequence_init(&message_sequence, 0)); + EXPECT_EQ(0u, message_sequence.size); + EXPECT_EQ(0u, message_sequence.capacity); + EXPECT_EQ(nullptr, message_sequence.data); + + EXPECT_EQ(RMW_RET_OK, rmw_message_sequence_fini(&message_sequence)); + EXPECT_EQ(0u, message_sequence.size); + EXPECT_EQ(0u, message_sequence.capacity); + EXPECT_EQ(nullptr, message_sequence.data); +} + +TEST(test_message_sequence, initialization_with_size) { + auto message_sequence = rmw_get_zero_initialized_message_sequence(); + + EXPECT_EQ(RMW_RET_OK, rmw_message_sequence_init(&message_sequence, 5)); + EXPECT_EQ(0u, message_sequence.size); + EXPECT_EQ(5u, message_sequence.capacity); + + EXPECT_EQ(RMW_RET_OK, rmw_message_sequence_fini(&message_sequence)); + EXPECT_EQ(0u, message_sequence.size); + EXPECT_EQ(0u, message_sequence.capacity); + EXPECT_EQ(nullptr, message_sequence.data); +} From fe2c074443335916cccf8cf79417cc862889b4d2 Mon Sep 17 00:00:00 2001 From: Michael Carroll Date: Wed, 15 Apr 2020 14:29:52 -0500 Subject: [PATCH 2/8] Apply suggestions from code review Co-Authored-By: Jacob Perron Signed-off-by: Michael Carroll --- rmw/src/message_sequence.c | 10 +++++----- rmw/test/test_message_sequence.cpp | 14 +++++++------- 2 files changed, 12 insertions(+), 12 deletions(-) diff --git a/rmw/src/message_sequence.c b/rmw/src/message_sequence.c index c208fa06..b4a1d7f6 100644 --- a/rmw/src/message_sequence.c +++ b/rmw/src/message_sequence.c @@ -57,8 +57,8 @@ rmw_message_sequence_fini( assert(sequence->capacity > 0); free(sequence->data); sequence->data = NULL; - sequence->size = 0; - sequence->capacity = 0; + sequence->size = 0u; + sequence->capacity = 0u; } else { assert(0 == sequence->size); assert(0 == sequence->capacity); @@ -93,7 +93,7 @@ rmw_message_info_sequence_init( } } sequence->data = data; - sequence->size = 0; + sequence->size = 0u; sequence->capacity = size; return RMW_RET_OK; } @@ -108,8 +108,8 @@ rmw_message_info_sequence_fini( assert(sequence->capacity > 0); free(sequence->data); sequence->data = NULL; - sequence->size = 0; - sequence->capacity = 0; + sequence->size = 0u; + sequence->capacity = 0u; } else { assert(0 == sequence->size); assert(0 == sequence->capacity); diff --git a/rmw/test/test_message_sequence.cpp b/rmw/test/test_message_sequence.cpp index 292cd83a..a5f50c46 100644 --- a/rmw/test/test_message_sequence.cpp +++ b/rmw/test/test_message_sequence.cpp @@ -1,4 +1,4 @@ -// Copyright 2018 Open Source Robotics Foundation, Inc. +// Copyright 2020 Open Source Robotics Foundation, Inc. // // Licensed under the Apache License, Version 2.0 (the "License"); // you may not use this file except in compliance with the License. @@ -22,7 +22,7 @@ TEST(test_message_info_sequence, default_initialization) { auto info_sequence = rmw_get_zero_initialized_message_info_sequence(); // Initialize the message sequence with a zero length - EXPECT_EQ(RMW_RET_OK, rmw_message_info_sequence_init(&info_sequence, 0)); + EXPECT_EQ(RMW_RET_OK, rmw_message_info_sequence_init(&info_sequence, 0u)); EXPECT_EQ(0u, info_sequence.size); EXPECT_EQ(0u, info_sequence.capacity); EXPECT_EQ(nullptr, info_sequence.data); @@ -36,10 +36,10 @@ TEST(test_message_info_sequence, default_initialization) { TEST(test_message_info_sequence, initialization_with_size) { auto info_sequence = rmw_get_zero_initialized_message_info_sequence(); - EXPECT_EQ(RMW_RET_OK, rmw_message_info_sequence_init(&info_sequence, 5)); + EXPECT_EQ(RMW_RET_OK, rmw_message_info_sequence_init(&info_sequence, 5u)); EXPECT_EQ(0u, info_sequence.size); EXPECT_EQ(5u, info_sequence.capacity); - +EXPECT_NE(nullptr, info_sequence.data); EXPECT_EQ(RMW_RET_OK, rmw_message_info_sequence_fini(&info_sequence)); EXPECT_EQ(0u, info_sequence.size); EXPECT_EQ(0u, info_sequence.capacity); @@ -50,7 +50,7 @@ TEST(test_message_sequence, default_initialization) { auto message_sequence = rmw_get_zero_initialized_message_sequence(); // Initialize the message sequence with a zero length - EXPECT_EQ(RMW_RET_OK, rmw_message_sequence_init(&message_sequence, 0)); + EXPECT_EQ(RMW_RET_OK, rmw_message_sequence_init(&message_sequence, 0u)); EXPECT_EQ(0u, message_sequence.size); EXPECT_EQ(0u, message_sequence.capacity); EXPECT_EQ(nullptr, message_sequence.data); @@ -64,10 +64,10 @@ TEST(test_message_sequence, default_initialization) { TEST(test_message_sequence, initialization_with_size) { auto message_sequence = rmw_get_zero_initialized_message_sequence(); - EXPECT_EQ(RMW_RET_OK, rmw_message_sequence_init(&message_sequence, 5)); + EXPECT_EQ(RMW_RET_OK, rmw_message_sequence_init(&message_sequence, 5u)); EXPECT_EQ(0u, message_sequence.size); EXPECT_EQ(5u, message_sequence.capacity); - +EXPECT_NE(nullptr, info_sequence.data); EXPECT_EQ(RMW_RET_OK, rmw_message_sequence_fini(&message_sequence)); EXPECT_EQ(0u, message_sequence.size); EXPECT_EQ(0u, message_sequence.capacity); From 83cd71b4cf64fb3d78bd92e07a4f2c94b9d6ede7 Mon Sep 17 00:00:00 2001 From: Michael Carroll Date: Wed, 15 Apr 2020 14:35:41 -0500 Subject: [PATCH 3/8] Clarify rmw_take_sequence arguments Signed-off-by: Michael Carroll --- rmw/include/rmw/rmw.h | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/rmw/include/rmw/rmw.h b/rmw/include/rmw/rmw.h index d70e642d..ab108aba 100644 --- a/rmw/include/rmw/rmw.h +++ b/rmw/include/rmw/rmw.h @@ -724,14 +724,24 @@ rmw_take_with_info( * While `count` messages may be requested, fewer messages may be available on the subscription. * In this case, only the currently available messages will be returned. * The `taken` flag indicate the number of messages actually taken. + * The method will return `RMW_RET_OK` even in the case that fewer (or zero) messages retrieved + * from the subscription, and will `RMW_RET_ERROR` in the case of unexpected errors. + * In the case that `count` is zero, the function will return `RMW_RET_INVALID_ARGUMENT`. + * + * `message_sequence` and `message_info_sequence` should be initialized and have sufficient capacity. + * It is not critical that the sequence sizes match, and they may be reused from previous calls. + * Both must be valid (not NULL) for the method to run successfully. * * \param[in] subscription The subscription object to take from. * \param[in] count Number of messages to attempt to take. * \param[out] message_sequence The sequence ROS message data on success. - * \param[out] message_info_sequence The seqeucne of additional message metadata on success. + * \param[out] message_info_sequence The sequence of additional message metadata on success. * \param[out] taken Number of messages actually taken from subscription. * \param[in] allocation Preallocated buffer to use (may be NULL). * \return `RMW_RET_OK` if successful, or + * \return `RMW_RET_INVALID_ARGUMENT` if an argument is invalid, or + * \return `RMW_RET_BAD_ALLOC` if memory allocation failed, or + * \return `RMW_RET_INCORRECT_RMW_IMPLEMENTATION` if the subscription is invalid, or * \return `RMW_RET_ERROR` if an unexpected error occurs. */ RMW_PUBLIC From cd4b65d727de6d4a98b12e2851caa7ea168d8515 Mon Sep 17 00:00:00 2001 From: Michael Carroll Date: Thu, 16 Apr 2020 10:55:14 -0500 Subject: [PATCH 4/8] Add support for using rcutils allocator Signed-off-by: Michael Carroll --- rmw/include/rmw/message_sequence.h | 59 ++++++++++++++++++++++++++++-- rmw/src/message_sequence.c | 27 ++++++++++---- rmw/test/test_message_sequence.cpp | 26 ++++++++----- 3 files changed, 90 insertions(+), 22 deletions(-) diff --git a/rmw/include/rmw/message_sequence.h b/rmw/include/rmw/message_sequence.h index e8707593..af323b44 100644 --- a/rmw/include/rmw/message_sequence.h +++ b/rmw/include/rmw/message_sequence.h @@ -26,17 +26,63 @@ extern "C" { #endif +/// Structure to hold a sequence of ROS messages. +typedef struct RMW_PUBLIC_TYPE rmw_message_sequence_t +{ + /// Array of pointers to ROS messages. + void ** data; + /// The number of valid entries in `data`. + size_t size; + /// The total allocated capacity of the data array. + size_t capacity; +} rmw_message_sequence_t; + +/// Structure to hold a sequence of message infos. +typedef struct RMW_PUBLIC_TYPE rmw_message_info_sequence_t +{ + /// Array of message info. + rmw_message_info_t * data; + /// The number of valid entries in data. + size_t size; + /// The total allocated capacity of the data array. + size_t capacity; +} rmw_message_info_sequence_t; + +/// Return an rmw_message_sequence_t struct with members initialized to `NULL`; RMW_PUBLIC rmw_message_sequence_t rmw_get_zero_initialized_message_sequence(void); +/// Initialize an rmw_message_sequence_t object. +/** + * \param[inout] sequence sequence object to be initialized. + * \param[in] size capacity of the sequence to be allocated. + * \param[in] allocator the allcator used to allocate memory. + */ RMW_PUBLIC rmw_ret_t -rmw_message_sequence_init(rmw_message_sequence_t * sequence, size_t size); +rmw_message_sequence_init( + rmw_message_sequence_t * sequence, + size_t size, + const rcutils_allocator_t * allocator); +/// Finalize an rmw_message_sequence_t object. +/** + * The rmw_message_sequence_t struct has members which require memory to be allocated to them + * before setting values. + * This function reclaims any allocated resources within the obect and zeroes out all other + * members. + * + * Note: This will not call `fini` or deallocate the underlying message structures. + * + * \param[inout] sequence sequence object to be finalized. + * \param[in] allocator the allocator used to allocate memory to the object. + */ RMW_PUBLIC rmw_ret_t -rmw_message_sequence_fini(rmw_message_sequence_t * sequence); +rmw_message_sequence_fini( + rmw_message_sequence_t * sequence, + const rcutils_allocator_t * allocator); RMW_PUBLIC rmw_message_info_sequence_t @@ -44,11 +90,16 @@ rmw_get_zero_initialized_message_info_sequence(void); RMW_PUBLIC rmw_ret_t -rmw_message_info_sequence_init(rmw_message_info_sequence_t * sequence, size_t size); +rmw_message_info_sequence_init( + rmw_message_info_sequence_t * sequence, + size_t size, + const rcutils_allocator_t * allocator); RMW_PUBLIC rmw_ret_t -rmw_message_info_sequence_fini(rmw_message_info_sequence_t * sequence); +rmw_message_info_sequence_fini( + rmw_message_info_sequence_t * sequence, + const rcutils_allocator_t * allocator); #if __cplusplus } diff --git a/rmw/src/message_sequence.c b/rmw/src/message_sequence.c index b4a1d7f6..37f98643 100644 --- a/rmw/src/message_sequence.c +++ b/rmw/src/message_sequence.c @@ -13,6 +13,7 @@ // limitations under the License. #include "rmw/message_sequence.h" +#include "rmw/types.h" rmw_message_sequence_t rmw_get_zero_initialized_message_sequence(void) @@ -28,13 +29,16 @@ rmw_get_zero_initialized_message_sequence(void) rmw_ret_t rmw_message_sequence_init( - rmw_message_sequence_t * sequence, size_t size) + rmw_message_sequence_t * sequence, + size_t size, + const rcutils_allocator_t * allocator) { RCUTILS_CHECK_ARGUMENT_FOR_NULL(sequence, RMW_RET_INVALID_ARGUMENT); + RCUTILS_CHECK_ALLOCATOR(allocator, return RMW_RET_INVALID_ARGUMENT); void * data = NULL; if (size) { - data = malloc(sizeof(void *) * size); + data = allocator->allocate(sizeof(void *) * size, allocator->state); if (!data) { return RMW_RET_BAD_ALLOC; } @@ -49,13 +53,15 @@ rmw_message_sequence_init( rmw_ret_t rmw_message_sequence_fini( - rmw_message_sequence_t * sequence) + rmw_message_sequence_t * sequence, + const rcutils_allocator_t * allocator) { RCUTILS_CHECK_ARGUMENT_FOR_NULL(sequence, RMW_RET_INVALID_ARGUMENT); + RCUTILS_CHECK_ALLOCATOR(allocator, return RMW_RET_INVALID_ARGUMENT); if (sequence->data) { assert(sequence->capacity > 0); - free(sequence->data); + allocator->deallocate(sequence->data, allocator->state); sequence->data = NULL; sequence->size = 0u; sequence->capacity = 0u; @@ -81,13 +87,16 @@ rmw_get_zero_initialized_message_info_sequence(void) rmw_ret_t rmw_message_info_sequence_init( - rmw_message_info_sequence_t * sequence, size_t size) + rmw_message_info_sequence_t * sequence, + size_t size, + const rcutils_allocator_t * allocator) { RCUTILS_CHECK_ARGUMENT_FOR_NULL(sequence, RMW_RET_INVALID_ARGUMENT); + RCUTILS_CHECK_ALLOCATOR(allocator, return RMW_RET_INVALID_ARGUMENT); rmw_message_info_t * data = NULL; if (size) { - data = malloc(sizeof(rmw_message_info_t) * size); + data = allocator->allocate(sizeof(rmw_message_info_t) * size, allocator->state); if (!data) { return RMW_RET_BAD_ALLOC; } @@ -100,13 +109,15 @@ rmw_message_info_sequence_init( rmw_ret_t rmw_message_info_sequence_fini( - rmw_message_info_sequence_t * sequence) + rmw_message_info_sequence_t * sequence, + const rcutils_allocator_t * allocator) { RCUTILS_CHECK_ARGUMENT_FOR_NULL(sequence, RMW_RET_INVALID_ARGUMENT); + RCUTILS_CHECK_ALLOCATOR(allocator, return RMW_RET_INVALID_ARGUMENT); if (sequence->data) { assert(sequence->capacity > 0); - free(sequence->data); + allocator->deallocate(sequence->data, allocator->state); sequence->data = NULL; sequence->size = 0u; sequence->capacity = 0u; diff --git a/rmw/test/test_message_sequence.cpp b/rmw/test/test_message_sequence.cpp index a5f50c46..505050c3 100644 --- a/rmw/test/test_message_sequence.cpp +++ b/rmw/test/test_message_sequence.cpp @@ -20,14 +20,15 @@ TEST(test_message_info_sequence, default_initialization) { auto info_sequence = rmw_get_zero_initialized_message_info_sequence(); + auto allocator = rcutils_get_default_allocator(); // Initialize the message sequence with a zero length - EXPECT_EQ(RMW_RET_OK, rmw_message_info_sequence_init(&info_sequence, 0u)); + EXPECT_EQ(RMW_RET_OK, rmw_message_info_sequence_init(&info_sequence, 0u, &allocator)); EXPECT_EQ(0u, info_sequence.size); EXPECT_EQ(0u, info_sequence.capacity); EXPECT_EQ(nullptr, info_sequence.data); - EXPECT_EQ(RMW_RET_OK, rmw_message_info_sequence_fini(&info_sequence)); + EXPECT_EQ(RMW_RET_OK, rmw_message_info_sequence_fini(&info_sequence, &allocator)); EXPECT_EQ(0u, info_sequence.size); EXPECT_EQ(0u, info_sequence.capacity); EXPECT_EQ(nullptr, info_sequence.data); @@ -35,12 +36,14 @@ TEST(test_message_info_sequence, default_initialization) { TEST(test_message_info_sequence, initialization_with_size) { auto info_sequence = rmw_get_zero_initialized_message_info_sequence(); + auto allocator = rcutils_get_default_allocator(); - EXPECT_EQ(RMW_RET_OK, rmw_message_info_sequence_init(&info_sequence, 5u)); + EXPECT_EQ(RMW_RET_OK, rmw_message_info_sequence_init(&info_sequence, 5u, &allocator)); EXPECT_EQ(0u, info_sequence.size); EXPECT_EQ(5u, info_sequence.capacity); -EXPECT_NE(nullptr, info_sequence.data); - EXPECT_EQ(RMW_RET_OK, rmw_message_info_sequence_fini(&info_sequence)); + EXPECT_NE(nullptr, info_sequence.data); + + EXPECT_EQ(RMW_RET_OK, rmw_message_info_sequence_fini(&info_sequence, &allocator)); EXPECT_EQ(0u, info_sequence.size); EXPECT_EQ(0u, info_sequence.capacity); EXPECT_EQ(nullptr, info_sequence.data); @@ -48,14 +51,15 @@ EXPECT_NE(nullptr, info_sequence.data); TEST(test_message_sequence, default_initialization) { auto message_sequence = rmw_get_zero_initialized_message_sequence(); + auto allocator = rcutils_get_default_allocator(); // Initialize the message sequence with a zero length - EXPECT_EQ(RMW_RET_OK, rmw_message_sequence_init(&message_sequence, 0u)); + EXPECT_EQ(RMW_RET_OK, rmw_message_sequence_init(&message_sequence, 0u, &allocator)); EXPECT_EQ(0u, message_sequence.size); EXPECT_EQ(0u, message_sequence.capacity); EXPECT_EQ(nullptr, message_sequence.data); - EXPECT_EQ(RMW_RET_OK, rmw_message_sequence_fini(&message_sequence)); + EXPECT_EQ(RMW_RET_OK, rmw_message_sequence_fini(&message_sequence, &allocator)); EXPECT_EQ(0u, message_sequence.size); EXPECT_EQ(0u, message_sequence.capacity); EXPECT_EQ(nullptr, message_sequence.data); @@ -63,12 +67,14 @@ TEST(test_message_sequence, default_initialization) { TEST(test_message_sequence, initialization_with_size) { auto message_sequence = rmw_get_zero_initialized_message_sequence(); + auto allocator = rcutils_get_default_allocator(); - EXPECT_EQ(RMW_RET_OK, rmw_message_sequence_init(&message_sequence, 5u)); + EXPECT_EQ(RMW_RET_OK, rmw_message_sequence_init(&message_sequence, 5u, &allocator)); EXPECT_EQ(0u, message_sequence.size); EXPECT_EQ(5u, message_sequence.capacity); -EXPECT_NE(nullptr, info_sequence.data); - EXPECT_EQ(RMW_RET_OK, rmw_message_sequence_fini(&message_sequence)); + EXPECT_NE(nullptr, message_sequence.data); + + EXPECT_EQ(RMW_RET_OK, rmw_message_sequence_fini(&message_sequence, &allocator)); EXPECT_EQ(0u, message_sequence.size); EXPECT_EQ(0u, message_sequence.capacity); EXPECT_EQ(nullptr, message_sequence.data); From 0eb650ee1de399838014b26b64a534c14612bf0a Mon Sep 17 00:00:00 2001 From: Michael Carroll Date: Thu, 16 Apr 2020 11:03:42 -0500 Subject: [PATCH 5/8] Store allocator inside sequence structure Signed-off-by: Michael Carroll --- rmw/include/rmw/message_sequence.h | 17 +++++------ rmw/src/message_sequence.c | 48 ++++++++++++++---------------- rmw/test/test_message_sequence.cpp | 8 ++--- 3 files changed, 35 insertions(+), 38 deletions(-) diff --git a/rmw/include/rmw/message_sequence.h b/rmw/include/rmw/message_sequence.h index af323b44..cc032e82 100644 --- a/rmw/include/rmw/message_sequence.h +++ b/rmw/include/rmw/message_sequence.h @@ -35,6 +35,8 @@ typedef struct RMW_PUBLIC_TYPE rmw_message_sequence_t size_t size; /// The total allocated capacity of the data array. size_t capacity; + /// The allocator used to allocate the data array. + rcutils_allocator_t * allocator; } rmw_message_sequence_t; /// Structure to hold a sequence of message infos. @@ -46,6 +48,8 @@ typedef struct RMW_PUBLIC_TYPE rmw_message_info_sequence_t size_t size; /// The total allocated capacity of the data array. size_t capacity; + /// The allocator used to allocate the data array. + rcutils_allocator_t * allocator; } rmw_message_info_sequence_t; /// Return an rmw_message_sequence_t struct with members initialized to `NULL`; @@ -64,7 +68,7 @@ rmw_ret_t rmw_message_sequence_init( rmw_message_sequence_t * sequence, size_t size, - const rcutils_allocator_t * allocator); + rcutils_allocator_t * allocator); /// Finalize an rmw_message_sequence_t object. /** @@ -76,13 +80,10 @@ rmw_message_sequence_init( * Note: This will not call `fini` or deallocate the underlying message structures. * * \param[inout] sequence sequence object to be finalized. - * \param[in] allocator the allocator used to allocate memory to the object. */ RMW_PUBLIC rmw_ret_t -rmw_message_sequence_fini( - rmw_message_sequence_t * sequence, - const rcutils_allocator_t * allocator); +rmw_message_sequence_fini(rmw_message_sequence_t * sequence); RMW_PUBLIC rmw_message_info_sequence_t @@ -93,13 +94,11 @@ rmw_ret_t rmw_message_info_sequence_init( rmw_message_info_sequence_t * sequence, size_t size, - const rcutils_allocator_t * allocator); + rcutils_allocator_t * allocator); RMW_PUBLIC rmw_ret_t -rmw_message_info_sequence_fini( - rmw_message_info_sequence_t * sequence, - const rcutils_allocator_t * allocator); +rmw_message_info_sequence_fini(rmw_message_info_sequence_t * sequence); #if __cplusplus } diff --git a/rmw/src/message_sequence.c b/rmw/src/message_sequence.c index 37f98643..6045d8d1 100644 --- a/rmw/src/message_sequence.c +++ b/rmw/src/message_sequence.c @@ -31,7 +31,7 @@ rmw_ret_t rmw_message_sequence_init( rmw_message_sequence_t * sequence, size_t size, - const rcutils_allocator_t * allocator) + rcutils_allocator_t * allocator) { RCUTILS_CHECK_ARGUMENT_FOR_NULL(sequence, RMW_RET_INVALID_ARGUMENT); RCUTILS_CHECK_ALLOCATOR(allocator, return RMW_RET_INVALID_ARGUMENT); @@ -47,29 +47,27 @@ rmw_message_sequence_init( sequence->data = data; sequence->size = 0; sequence->capacity = size; + sequence->allocator = allocator; return RMW_RET_OK; } rmw_ret_t -rmw_message_sequence_fini( - rmw_message_sequence_t * sequence, - const rcutils_allocator_t * allocator) +rmw_message_sequence_fini(rmw_message_sequence_t * sequence) { RCUTILS_CHECK_ARGUMENT_FOR_NULL(sequence, RMW_RET_INVALID_ARGUMENT); - RCUTILS_CHECK_ALLOCATOR(allocator, return RMW_RET_INVALID_ARGUMENT); if (sequence->data) { assert(sequence->capacity > 0); - allocator->deallocate(sequence->data, allocator->state); - sequence->data = NULL; - sequence->size = 0u; - sequence->capacity = 0u; - } else { - assert(0 == sequence->size); - assert(0 == sequence->capacity); + RCUTILS_CHECK_ALLOCATOR(sequence->allocator, return RMW_RET_INVALID_ARGUMENT); + sequence->allocator->deallocate(sequence->data, sequence->allocator->state); } + sequence->data = NULL; + sequence->size = 0u; + sequence->capacity = 0u; + sequence->allocator = NULL; + return RMW_RET_OK; } @@ -79,7 +77,8 @@ rmw_get_zero_initialized_message_info_sequence(void) static rmw_message_info_sequence_t message_info_sequence = { .data = NULL, .size = 0u, - .capacity = 0u + .capacity = 0u, + .allocator = NULL }; return message_info_sequence; @@ -89,7 +88,7 @@ rmw_ret_t rmw_message_info_sequence_init( rmw_message_info_sequence_t * sequence, size_t size, - const rcutils_allocator_t * allocator) + rcutils_allocator_t * allocator) { RCUTILS_CHECK_ARGUMENT_FOR_NULL(sequence, RMW_RET_INVALID_ARGUMENT); RCUTILS_CHECK_ALLOCATOR(allocator, return RMW_RET_INVALID_ARGUMENT); @@ -104,27 +103,26 @@ rmw_message_info_sequence_init( sequence->data = data; sequence->size = 0u; sequence->capacity = size; + sequence->allocator = allocator; + return RMW_RET_OK; } rmw_ret_t -rmw_message_info_sequence_fini( - rmw_message_info_sequence_t * sequence, - const rcutils_allocator_t * allocator) +rmw_message_info_sequence_fini(rmw_message_info_sequence_t * sequence) { RCUTILS_CHECK_ARGUMENT_FOR_NULL(sequence, RMW_RET_INVALID_ARGUMENT); - RCUTILS_CHECK_ALLOCATOR(allocator, return RMW_RET_INVALID_ARGUMENT); if (sequence->data) { assert(sequence->capacity > 0); - allocator->deallocate(sequence->data, allocator->state); - sequence->data = NULL; - sequence->size = 0u; - sequence->capacity = 0u; - } else { - assert(0 == sequence->size); - assert(0 == sequence->capacity); + RCUTILS_CHECK_ALLOCATOR(sequence->allocator, return RMW_RET_INVALID_ARGUMENT); + sequence->allocator->deallocate(sequence->data, sequence->allocator->state); } + sequence->data = NULL; + sequence->size = 0u; + sequence->capacity = 0u; + sequence->allocator = NULL; + return RMW_RET_OK; } diff --git a/rmw/test/test_message_sequence.cpp b/rmw/test/test_message_sequence.cpp index 505050c3..bfaf6e53 100644 --- a/rmw/test/test_message_sequence.cpp +++ b/rmw/test/test_message_sequence.cpp @@ -28,7 +28,7 @@ TEST(test_message_info_sequence, default_initialization) { EXPECT_EQ(0u, info_sequence.capacity); EXPECT_EQ(nullptr, info_sequence.data); - EXPECT_EQ(RMW_RET_OK, rmw_message_info_sequence_fini(&info_sequence, &allocator)); + EXPECT_EQ(RMW_RET_OK, rmw_message_info_sequence_fini(&info_sequence)); EXPECT_EQ(0u, info_sequence.size); EXPECT_EQ(0u, info_sequence.capacity); EXPECT_EQ(nullptr, info_sequence.data); @@ -43,7 +43,7 @@ TEST(test_message_info_sequence, initialization_with_size) { EXPECT_EQ(5u, info_sequence.capacity); EXPECT_NE(nullptr, info_sequence.data); - EXPECT_EQ(RMW_RET_OK, rmw_message_info_sequence_fini(&info_sequence, &allocator)); + EXPECT_EQ(RMW_RET_OK, rmw_message_info_sequence_fini(&info_sequence)); EXPECT_EQ(0u, info_sequence.size); EXPECT_EQ(0u, info_sequence.capacity); EXPECT_EQ(nullptr, info_sequence.data); @@ -59,7 +59,7 @@ TEST(test_message_sequence, default_initialization) { EXPECT_EQ(0u, message_sequence.capacity); EXPECT_EQ(nullptr, message_sequence.data); - EXPECT_EQ(RMW_RET_OK, rmw_message_sequence_fini(&message_sequence, &allocator)); + EXPECT_EQ(RMW_RET_OK, rmw_message_sequence_fini(&message_sequence)); EXPECT_EQ(0u, message_sequence.size); EXPECT_EQ(0u, message_sequence.capacity); EXPECT_EQ(nullptr, message_sequence.data); @@ -74,7 +74,7 @@ TEST(test_message_sequence, initialization_with_size) { EXPECT_EQ(5u, message_sequence.capacity); EXPECT_NE(nullptr, message_sequence.data); - EXPECT_EQ(RMW_RET_OK, rmw_message_sequence_fini(&message_sequence, &allocator)); + EXPECT_EQ(RMW_RET_OK, rmw_message_sequence_fini(&message_sequence)); EXPECT_EQ(0u, message_sequence.size); EXPECT_EQ(0u, message_sequence.capacity); EXPECT_EQ(nullptr, message_sequence.data); From 55f93d335d9622a0e4b3b6df8f017af125f10755 Mon Sep 17 00:00:00 2001 From: Michael Carroll Date: Mon, 20 Apr 2020 14:30:08 -0500 Subject: [PATCH 6/8] Apply suggestions from code review Co-Authored-By: Jacob Perron Co-Authored-By: William Woodall Signed-off-by: Michael Carroll --- rmw/include/rmw/message_sequence.h | 4 ++-- rmw/include/rmw/rmw.h | 8 ++++---- rmw/src/message_sequence.c | 3 ++- 3 files changed, 8 insertions(+), 7 deletions(-) diff --git a/rmw/include/rmw/message_sequence.h b/rmw/include/rmw/message_sequence.h index cc032e82..35e857ae 100644 --- a/rmw/include/rmw/message_sequence.h +++ b/rmw/include/rmw/message_sequence.h @@ -52,7 +52,7 @@ typedef struct RMW_PUBLIC_TYPE rmw_message_info_sequence_t rcutils_allocator_t * allocator; } rmw_message_info_sequence_t; -/// Return an rmw_message_sequence_t struct with members initialized to `NULL`; +/// Return an rmw_message_sequence_t struct with members initialized to `NULL` RMW_PUBLIC rmw_message_sequence_t rmw_get_zero_initialized_message_sequence(void); @@ -74,7 +74,7 @@ rmw_message_sequence_init( /** * The rmw_message_sequence_t struct has members which require memory to be allocated to them * before setting values. - * This function reclaims any allocated resources within the obect and zeroes out all other + * This function reclaims any allocated resources within the object and zeroes out all other * members. * * Note: This will not call `fini` or deallocate the underlying message structures. diff --git a/rmw/include/rmw/rmw.h b/rmw/include/rmw/rmw.h index ab108aba..9e1c9fb1 100644 --- a/rmw/include/rmw/rmw.h +++ b/rmw/include/rmw/rmw.h @@ -723,8 +723,8 @@ rmw_take_with_info( * * While `count` messages may be requested, fewer messages may be available on the subscription. * In this case, only the currently available messages will be returned. - * The `taken` flag indicate the number of messages actually taken. - * The method will return `RMW_RET_OK` even in the case that fewer (or zero) messages retrieved + * The `taken` flag indicates the number of messages actually taken. + * The method will return `RMW_RET_OK` even in the case that fewer (or zero) messages were retrieved. * from the subscription, and will `RMW_RET_ERROR` in the case of unexpected errors. * In the case that `count` is zero, the function will return `RMW_RET_INVALID_ARGUMENT`. * @@ -734,14 +734,14 @@ rmw_take_with_info( * * \param[in] subscription The subscription object to take from. * \param[in] count Number of messages to attempt to take. - * \param[out] message_sequence The sequence ROS message data on success. + * \param[out] message_sequence The sequence of ROS message data on success. * \param[out] message_info_sequence The sequence of additional message metadata on success. * \param[out] taken Number of messages actually taken from subscription. * \param[in] allocation Preallocated buffer to use (may be NULL). * \return `RMW_RET_OK` if successful, or * \return `RMW_RET_INVALID_ARGUMENT` if an argument is invalid, or * \return `RMW_RET_BAD_ALLOC` if memory allocation failed, or - * \return `RMW_RET_INCORRECT_RMW_IMPLEMENTATION` if the subscription is invalid, or + * \return `RMW_RET_INCORRECT_RMW_IMPLEMENTATION` if the rmw implementation does not match, or * \return `RMW_RET_ERROR` if an unexpected error occurs. */ RMW_PUBLIC diff --git a/rmw/src/message_sequence.c b/rmw/src/message_sequence.c index 6045d8d1..0ba3ca67 100644 --- a/rmw/src/message_sequence.c +++ b/rmw/src/message_sequence.c @@ -21,7 +21,8 @@ rmw_get_zero_initialized_message_sequence(void) static rmw_message_sequence_t message_sequence = { .data = NULL, .size = 0u, - .capacity = 0u + .capacity = 0u, + .allocator = NULL }; return message_sequence; From e94d679b9d37e3a1b4acba509f918385e625bc0f Mon Sep 17 00:00:00 2001 From: Michael Carroll Date: Mon, 20 Apr 2020 14:31:35 -0500 Subject: [PATCH 7/8] Update docs per reviewers Signed-off-by: Michael Carroll --- rmw/include/rmw/message_sequence.h | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/rmw/include/rmw/message_sequence.h b/rmw/include/rmw/message_sequence.h index 35e857ae..d8e2bad5 100644 --- a/rmw/include/rmw/message_sequence.h +++ b/rmw/include/rmw/message_sequence.h @@ -85,10 +85,17 @@ RMW_PUBLIC rmw_ret_t rmw_message_sequence_fini(rmw_message_sequence_t * sequence); +/// Return an rmw_message_info_sequence_t struct with members initialized to `NULL` RMW_PUBLIC rmw_message_info_sequence_t rmw_get_zero_initialized_message_info_sequence(void); +/// Initialize an rmw_message_info_sequence_t object. +/** + * \param[inout] sequence sequence object to be initialized. + * \param[in] size capacity of the sequence to be allocated. + * \param[in] allocator the allcator used to allocate memory. + */ RMW_PUBLIC rmw_ret_t rmw_message_info_sequence_init( @@ -96,6 +103,15 @@ rmw_message_info_sequence_init( size_t size, rcutils_allocator_t * allocator); +/// Finalize an rmw_message_sequence_t object. +/** + * The rmw_message_sequence_t struct has members which require memory to be allocated to them + * before setting values. + * This function reclaims any allocated resources within the object and zeroes out all other + * members. + * + * \param[inout] sequence sequence object to be finalized. + */ RMW_PUBLIC rmw_ret_t rmw_message_info_sequence_fini(rmw_message_info_sequence_t * sequence); From 40d5457fa65d6ff4db37462618dd13e0111e60a3 Mon Sep 17 00:00:00 2001 From: Michael Carroll Date: Thu, 23 Apr 2020 16:48:58 -0500 Subject: [PATCH 8/8] Address reviewer feedback Signed-off-by: Michael Carroll --- rmw/src/message_sequence.c | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/rmw/src/message_sequence.c b/rmw/src/message_sequence.c index 0ba3ca67..373feb02 100644 --- a/rmw/src/message_sequence.c +++ b/rmw/src/message_sequence.c @@ -38,15 +38,15 @@ rmw_message_sequence_init( RCUTILS_CHECK_ALLOCATOR(allocator, return RMW_RET_INVALID_ARGUMENT); void * data = NULL; - if (size) { + if (size > 0u) { data = allocator->allocate(sizeof(void *) * size, allocator->state); - if (!data) { + if (NULL == data) { return RMW_RET_BAD_ALLOC; } } sequence->data = data; - sequence->size = 0; + sequence->size = 0u; sequence->capacity = size; sequence->allocator = allocator; @@ -58,8 +58,8 @@ rmw_message_sequence_fini(rmw_message_sequence_t * sequence) { RCUTILS_CHECK_ARGUMENT_FOR_NULL(sequence, RMW_RET_INVALID_ARGUMENT); - if (sequence->data) { - assert(sequence->capacity > 0); + if (NULL != sequence->data) { + assert(sequence->capacity > 0u); RCUTILS_CHECK_ALLOCATOR(sequence->allocator, return RMW_RET_INVALID_ARGUMENT); sequence->allocator->deallocate(sequence->data, sequence->allocator->state); } @@ -95,9 +95,9 @@ rmw_message_info_sequence_init( RCUTILS_CHECK_ALLOCATOR(allocator, return RMW_RET_INVALID_ARGUMENT); rmw_message_info_t * data = NULL; - if (size) { + if (size > 0u) { data = allocator->allocate(sizeof(rmw_message_info_t) * size, allocator->state); - if (!data) { + if (NULL == data) { return RMW_RET_BAD_ALLOC; } } @@ -114,8 +114,8 @@ rmw_message_info_sequence_fini(rmw_message_info_sequence_t * sequence) { RCUTILS_CHECK_ARGUMENT_FOR_NULL(sequence, RMW_RET_INVALID_ARGUMENT); - if (sequence->data) { - assert(sequence->capacity > 0); + if (NULL != sequence->data) { + assert(sequence->capacity > 0u); RCUTILS_CHECK_ALLOCATOR(sequence->allocator, return RMW_RET_INVALID_ARGUMENT); sequence->allocator->deallocate(sequence->data, sequence->allocator->state); }