Skip to content

Commit 9e98507

Browse files
jmachowinskiJanosch Machowinski
authored andcommitted
fix: Use default rcl allocator if allocator is std::allocator (#3069)
This fixes a bunch of warnings if using ASAN / valgrind on newer OS versions. It also fixed a real bug, as giving the wrong size on deallocate is undefined behavior according to the C++ standard. This version of the patch keeps the behavior for users that specified an own allocator the same and in therefore back portable. Signed-off-by: Janosch Machowinski <J.Machowinski@cellumation.com> Co-authored-by: Janosch Machowinski <j.machowinski@cellumation.com> (cherry picked from commit dc4a1db)
1 parent 1f66148 commit 9e98507

4 files changed

Lines changed: 30 additions & 7 deletions

File tree

rclcpp/include/rclcpp/message_memory_strategy.hpp

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@
1818
#include <memory>
1919
#include <stdexcept>
2020

21+
#include "rcl/allocator.h"
2122
#include "rcl/types.h"
2223

2324
#include "rclcpp/allocator/allocator_common.hpp"
@@ -61,15 +62,25 @@ class MessageMemoryStrategy
6162
message_allocator_ = std::make_shared<MessageAlloc>();
6263
serialized_message_allocator_ = std::make_shared<SerializedMessageAlloc>();
6364
buffer_allocator_ = std::make_shared<BufferAlloc>();
64-
rcutils_allocator_ = allocator::get_rcl_allocator<char, BufferAlloc>(*buffer_allocator_.get());
65+
if constexpr (std::is_same_v<Alloc, std::allocator<void>>) {
66+
rcutils_allocator_ = rcl_get_default_allocator();
67+
} else {
68+
rcutils_allocator_ = allocator::get_rcl_allocator<char,
69+
BufferAlloc>(*buffer_allocator_.get());
70+
}
6571
}
6672

6773
explicit MessageMemoryStrategy(std::shared_ptr<Alloc> allocator)
6874
{
6975
message_allocator_ = std::make_shared<MessageAlloc>(*allocator.get());
7076
serialized_message_allocator_ = std::make_shared<SerializedMessageAlloc>(*allocator.get());
7177
buffer_allocator_ = std::make_shared<BufferAlloc>(*allocator.get());
72-
rcutils_allocator_ = allocator::get_rcl_allocator<char, BufferAlloc>(*buffer_allocator_.get());
78+
if constexpr (std::is_same_v<Alloc, std::allocator<void>>) {
79+
rcutils_allocator_ = rcl_get_default_allocator();
80+
} else {
81+
rcutils_allocator_ = allocator::get_rcl_allocator<char,
82+
BufferAlloc>(*buffer_allocator_.get());
83+
}
7384
}
7485

7586
virtual ~MessageMemoryStrategy() = default;

rclcpp/include/rclcpp/publisher_options.hpp

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -119,6 +119,10 @@ struct PublisherOptionsWithAllocator : public PublisherOptionsBase
119119
rcl_allocator_t
120120
get_rcl_allocator() const
121121
{
122+
if constexpr (std::is_same_v<Allocator, std::allocator<void>>) {
123+
return rcl_get_default_allocator();
124+
}
125+
122126
if (!plain_allocator_storage_) {
123127
plain_allocator_storage_ =
124128
std::make_shared<PlainAllocator>(*this->get_allocator());

rclcpp/include/rclcpp/strategies/allocator_memory_strategy.hpp

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -424,7 +424,11 @@ class AllocatorMemoryStrategy : public memory_strategy::MemoryStrategy
424424

425425
rcl_allocator_t get_allocator() override
426426
{
427-
return rclcpp::allocator::get_rcl_allocator<void *, VoidAlloc>(*allocator_.get());
427+
if constexpr (std::is_same_v<Alloc, std::allocator<void>>) {
428+
return rcl_get_default_allocator();
429+
} else {
430+
return rclcpp::allocator::get_rcl_allocator<void *, VoidAlloc>(*allocator_.get());
431+
}
428432
}
429433

430434
size_t number_of_ready_subscriptions() const override

rclcpp/include/rclcpp/subscription_options.hpp

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -163,11 +163,15 @@ struct SubscriptionOptionsWithAllocator : public SubscriptionOptionsBase
163163
rcl_allocator_t
164164
get_rcl_allocator() const
165165
{
166-
if (!plain_allocator_storage_) {
167-
plain_allocator_storage_ =
168-
std::make_shared<PlainAllocator>(*this->get_allocator());
166+
if constexpr (std::is_same_v<Allocator, std::allocator<void>>) {
167+
return rcl_get_default_allocator();
168+
} else {
169+
if (!plain_allocator_storage_) {
170+
plain_allocator_storage_ =
171+
std::make_shared<PlainAllocator>(*this->get_allocator());
172+
}
173+
return rclcpp::allocator::get_rcl_allocator<char>(*plain_allocator_storage_);
169174
}
170-
return rclcpp::allocator::get_rcl_allocator<char>(*plain_allocator_storage_);
171175
}
172176

173177
// This is a temporal workaround, to make sure that get_allocator()

0 commit comments

Comments
 (0)