Skip to content

Commit dc4a1db

Browse files
jmachowinskiJanosch Machowinski
andauthored
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>
1 parent 6f96da6 commit dc4a1db

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
@@ -123,6 +123,10 @@ struct PublisherOptionsWithAllocator : public PublisherOptionsBase
123123
rcl_allocator_t
124124
get_rcl_allocator() const
125125
{
126+
if constexpr (std::is_same_v<Allocator, std::allocator<void>>) {
127+
return rcl_get_default_allocator();
128+
}
129+
126130
if (!plain_allocator_storage_) {
127131
plain_allocator_storage_ =
128132
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
@@ -426,7 +426,11 @@ class AllocatorMemoryStrategy : public memory_strategy::MemoryStrategy
426426

427427
rcl_allocator_t get_allocator() override
428428
{
429-
return rclcpp::allocator::get_rcl_allocator<void *, VoidAlloc>(*allocator_.get());
429+
if constexpr (std::is_same_v<Alloc, std::allocator<void>>) {
430+
return rcl_get_default_allocator();
431+
} else {
432+
return rclcpp::allocator::get_rcl_allocator<void *, VoidAlloc>(*allocator_.get());
433+
}
430434
}
431435

432436
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
@@ -167,11 +167,15 @@ struct SubscriptionOptionsWithAllocator : public SubscriptionOptionsBase
167167
rcl_allocator_t
168168
get_rcl_allocator() const
169169
{
170-
if (!plain_allocator_storage_) {
171-
plain_allocator_storage_ =
172-
std::make_shared<PlainAllocator>(*this->get_allocator());
170+
if constexpr (std::is_same_v<Allocator, std::allocator<void>>) {
171+
return rcl_get_default_allocator();
172+
} else {
173+
if (!plain_allocator_storage_) {
174+
plain_allocator_storage_ =
175+
std::make_shared<PlainAllocator>(*this->get_allocator());
176+
}
177+
return rclcpp::allocator::get_rcl_allocator<char>(*plain_allocator_storage_);
173178
}
174-
return rclcpp::allocator::get_rcl_allocator<char>(*plain_allocator_storage_);
175179
}
176180

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

0 commit comments

Comments
 (0)