get_all_data_impl() does not handle null pointers properly, causing segmentation fault (#2840)

* get_all_data_impl() does not handle null pointers properly, causing segmentation fault

Signed-off-by: Alejandro Hernandez Cordero <[email protected]>

* Update rclcpp/include/rclcpp/experimental/buffers/ring_buffer_implementation.hpp

Co-authored-by: Janosch Machowinski <[email protected]>
Signed-off-by: Alejandro Hernández Cordero <[email protected]>

* make linters happy

Signed-off-by: Alejandro Hernandez Cordero <[email protected]>

---------

Signed-off-by: Alejandro Hernandez Cordero <[email protected]>
Signed-off-by: Alejandro Hernández Cordero <[email protected]>
Co-authored-by: Janosch Machowinski <[email protected]>

Author: ahcorde

rclcpp/include/rclcpp/experimental/buffers/ring_buffer_implementation.hpp

@@ -265,9 +265,13 @@ class RingBufferImplementation : public BufferImplementationBase<BufferT>
     std::vector<BufferT> result_vtr;
     result_vtr.reserve(size_);
     for (size_t id = 0; id < size_; ++id) {
-      result_vtr.emplace_back(
-        new typename is_std_unique_ptr<T>::Ptr_type(
-          *(ring_buffer_[(read_index_ + id) % capacity_])));
+      const auto & elem(ring_buffer_[(read_index_ + id) % capacity_]);
+      if (elem != nullptr) {
+        result_vtr.emplace_back(new typename is_std_unique_ptr<T>::Ptr_type(
+          *elem));
+      } else {
+        result_vtr.emplace_back(nullptr);
+      }
     }
     return result_vtr;
   }

rclcpp/test/rclcpp/test_ring_buffer_implementation.cpp

@@ -164,3 +164,15 @@ TEST(TestRingBufferImplementation, test_buffer_clear) {
   EXPECT_EQ('c', c);
   EXPECT_EQ('d', d);
 }
+
+TEST(TestRingBufferImplementation, handle_nullptr_deletion) {
+  rclcpp::experimental::buffers::RingBufferImplementation<std::unique_ptr<int>> rb(3);
+  rb.enqueue(std::make_unique<int>(42));
+  rb.enqueue(nullptr);  // intentionally enqueuing nullptr
+  rb.enqueue(std::make_unique<int>(84));
+  auto all_data = rb.get_all_data();
+  EXPECT_EQ(3u, all_data.size());
+  EXPECT_EQ(42, *(all_data[0]));
+  EXPECT_EQ(nullptr, all_data[1]);
+  EXPECT_EQ(84, *(all_data[2]));
+}