iox-eclipse-iceoryx#218 Fix move constructor issue with sample.

Signed-off-by: Ithier Jeff (CC-AD/EYF1) <[email protected]>

iceoryx_examples/icedelivery/iox_publisher_modern.cpp

@@ -43,6 +43,7 @@ int main(int argc, char *argv[])
 {
  iox::runtime::PoshRuntime::getInstance("/iox-ex-publisher-modern");
  auto publisher = iox::popo::Publisher<Position>({"Odometry", "Position", "Vehicle"});
+ publisher.offer();
 
  float_t ct = 0.0;
  while(!killswitch)

iceoryx_posh/include/iceoryx_posh/experimental/internal/popo/publisher.inl

@@ -68,24 +68,26 @@ Publisher<T, port_t>::loan() noexcept
 }
 
 template<typename T, typename port_t>
-inline cxx::expected<Sample<T>, AllocationError>
-Publisher<T, port_t>::release(Sample<T>&& sample) noexcept
+inline cxx::expected<AllocationError>
+Publisher<T, port_t>::release(Sample<T>& sample) noexcept
 {
  auto header = iox::mepoo::convertPayloadPointerToChunkHeader(sample.allocation());
  m_port.freeChunk(header);
+ return iox::cxx::success<>();
 }
 
 template<typename T, typename port_t>
-inline cxx::expected<Sample<T>, AllocationError>
-Publisher<T, port_t>::publish(Sample<T>&& sample) noexcept
+inline cxx::expected<AllocationError>
+Publisher<T, port_t>::publish(Sample<T>& sample) noexcept
 {
  /// @todo - ensure sample points to valid shared memory location
  auto header = iox::mepoo::convertPayloadPointerToChunkHeader(reinterpret_cast<void* const>(sample.allocation()));
  m_port.deliverChunk(header);
+ return iox::cxx::success<>();
 }
 
 template<typename T, typename port_t>
-inline cxx::expected<Sample<T>, AllocationError>
+inline cxx::expected<AllocationError>
 Publisher<T, port_t>::publishResultOf(cxx::function_ref<void(T*)> f) noexcept
 {
  loan()
@@ -96,10 +98,11 @@ Publisher<T, port_t>::publishResultOf(cxx::function_ref<void(T*)> f) noexcept
 }
 
 template<typename T, typename port_t>
-inline cxx::expected<Sample<T>, AllocationError>
+inline cxx::expected<AllocationError>
 Publisher<T, port_t>::publishCopyOf(const T& val) noexcept
 {
  std::cout << "publishCopyOf()" << std::endl;
+ return iox::cxx::success<>();
 }
 
 template<typename T, typename port_t>
@@ -112,28 +115,28 @@ Publisher<T, port_t>::previous() const noexcept
 
 template<typename T, typename port_t>
 inline void
-Publisher<T, port_t>::offer() const noexcept
+Publisher<T, port_t>::offer() noexcept
 {
- m_port.offer();
+ m_port.activate();
 }
 
 template<typename T, typename port_t>
 inline void
-Publisher<T, port_t>::stopOffer() const noexcept
+Publisher<T, port_t>::stopOffer() noexcept
 {
- m_port.stopOffer();
+ m_port.deactivate();
 }
 
 template<typename T, typename port_t>
 inline bool
-Publisher<T, port_t>::isOffered() const noexcept
+Publisher<T, port_t>::isOffered() noexcept
 {
- return m_port.isOffered();
+ //return m_port.isOffered();
 }
 
 template<typename T, typename port_t>
 inline bool
-Publisher<T, port_t>::hasSubscribers() const noexcept
+Publisher<T, port_t>::hasSubscribers() noexcept
 {
  return m_port.hasSubscribers();
 }

iceoryx_posh/include/iceoryx_posh/experimental/popo/publisher.hpp

@@ -46,6 +46,11 @@ class Sample
  Sample(Sample&& rhs) = default;
  Sample& operator=(Sample&& rhs) = default;
 
+ ~Sample()
+ {
+ m_samplePtr = nullptr;
+ }
+
  T* operator->() noexcept
  {
  return m_samplePtr.get();
@@ -73,9 +78,9 @@ class Sample
  {
  if(m_isValid && !m_isEmpty)
  {
- m_publisher.publish(std::move(*this));
- m_samplePtr = nullptr;
- m_isValid = false;
+ m_publisher.publish(*this); // Only delivers chunk, doesn't modify pointer.
+ m_samplePtr.release(); // Release pointer, no deletion required.
+ m_isValid = false; // Mark as invalid to prevent re-use.
  }
  else
  {
@@ -134,21 +139,21 @@ class Publisher
  /// to it in the system.
  /// @param chunk
  ///
- cxx::expected<Sample<T>, AllocationError> release(Sample<T>&& sample) noexcept;
+ cxx::expected<AllocationError> release(Sample<T>& sample) noexcept;
 
  ///
  /// @brief send Publishes the loaned sample to all subscribers.
- /// @details The loanded sample is automatically released after publishing.
+ /// @details Only publishes, doesn't release chunk.
  /// @param chunk
  ///
- cxx::expected<Sample<T>, AllocationError> publish(Sample<T>&& sample) noexcept;
+ cxx::expected<AllocationError> publish(Sample<T>& sample) noexcept;
 
  ///
  /// @brief publish Publishes the argument produced by the given function.
  /// @details Sample is automatically loaned and released.
  /// @param f Function that produces a value T at the provided location.
  ///
- cxx::expected<Sample<T>, AllocationError> publishResultOf(cxx::function_ref<void(T*)> f) noexcept;
+ cxx::expected<AllocationError> publishResultOf(cxx::function_ref<void(T*)> f) noexcept;
 
  ///
  /// @brief copyAndPublish Copy the given sample into a loaned sample and publish it to all subscribers.
@@ -157,18 +162,19 @@ class Publisher
  /// rather than to write it elsewhere then copy it in.
  /// @param val The value to publish
  ///
- cxx::expected<Sample<T>, AllocationError> publishCopyOf(const T& val) noexcept;
+ cxx::expected<AllocationError> publishCopyOf(const T& val) noexcept;
 
  ///
  /// @brief previous Reclaims ownership of a previously published sample if it has not yet been accessed by subscribers.
  /// @return The previously published sample if one exists and is unclaimed, otherwise an error.
  ///
  cxx::expected<ChunkRecallError> previous() const noexcept;
 
- void offer() const noexcept;
- void stopOffer() const noexcept;
- bool isOffered() const noexcept;
- bool hasSubscribers() const noexcept;
+ /// @todo Make these const by changing the equivalent port methods.
+ void offer() noexcept;
+ void stopOffer() noexcept;
+ bool isOffered() noexcept;
+ bool hasSubscribers() noexcept;
 
 protected:
  port_t m_port{nullptr};

iceoryx_posh/test/moduletests/test_popo_publisher_experimental.cpp

@@ -39,6 +39,11 @@ class ExperimentalPublisherTest : public Test {
 
 };
 
+TEST_F(ExperimentalPublisherTest, OffersWhenPublishingOnUnofferedPublisher)
+{
+
+}
+
 //TEST_F(ExperimentalPublisherTest, BasicUidRetrieval)
 //{
 // struct Position {

iceoryx_utils/include/iceoryx_utils/cxx/unique_ptr.hpp

@@ -35,11 +35,21 @@ class unique_ptr{
 
  unique_ptr() = delete;
 
+// ///
+// /// @brief An empty pointer that does nothing.
+// ///
+// unique_ptr(std::nullptr_t) noexcept
+// {};
+
  ///
- /// @brief An empty pointer that does nothing.
+ /// @brief operator = Reset to empty pointer when setting to nullptr.
+ /// @return An empty unique pointer.
  ///
- unique_ptr(std::nullptr_t) noexcept
- {};
+ unique_ptr& operator=(std::nullptr_t) noexcept
+ {
+ reset();
+ return *this;
+ }
 
  ///
  /// @brief unique_ptr Creates an empty unique ptr that owns nothing. Can be passed ownership later.
@@ -84,16 +94,6 @@ class unique_ptr{
  explicit operator bool() const noexcept
  { return get() == ptr_t() ? false : true; }
 
- ///
- /// @brief operator = Reset to empty pointer when setting to nullptr.
- /// @return An empty unique pointer.
- ///
- unique_ptr& operator=(std::nullptr_t) noexcept
- {
- reset();
- return *this;
- }
-
  ///
  /// @brief get Retrieve the underlying raw pointer.
  /// @details The unique_ptr retains ownership, therefore the "borrowed" pointer must not be deleted.
@@ -109,10 +109,11 @@ class unique_ptr{
 
  ///
  /// @brief reset Reset the unique_ptr instance's owned object to the one given.
- /// @details Any previously owned objects will be deleted. If no pointer given, resets ot an empty pointer.
+ /// @details Any previously owned objects will be deleted. If no pointer given then points to nullptr.
+ /// Deleter provided on instantiation will remain.
  /// @param ptr Pointer to object to take ownership on.
  ///
- void reset(ptr_t ptr = ptr_t()) noexcept;
+ void reset(ptr_t ptr = nullptr) noexcept;
 
  ///
  /// @brief swap Swaps object ownership with another unique_ptr.
@@ -122,7 +123,6 @@ class unique_ptr{
 
 private:
  ptr_t m_ptr = nullptr;
-
  std::function<void(T*)> m_deleter = [](T* const){};
 };
 

iceoryx_utils/include/iceoryx_utils/internal/cxx/unique_ptr.inl

@@ -10,32 +10,42 @@ namespace cxx
 {
 
 template<typename T>
-unique_ptr<T>::unique_ptr(std::function<void(T*)>&& deleter) noexcept : m_deleter(std::move(deleter))
+unique_ptr<T>::unique_ptr(std::function<void(T*)>&& deleter) noexcept : m_deleter(deleter)
 {}
 
 template<typename T>
-unique_ptr<T>::unique_ptr(ptr_t ptr, std::function<void(T*)>&& deleter) noexcept : m_ptr(ptr), m_deleter(std::move(deleter))
+unique_ptr<T>::unique_ptr(ptr_t ptr, std::function<void(T*)>&& deleter) noexcept : m_ptr(ptr), m_deleter(deleter)
 {}
 
 template<typename T>
 unique_ptr<T>::unique_ptr(void* allocation, std::function<void(T*)>&& deleter) noexcept
- : m_ptr(reinterpret_cast<T*>(allocation)), m_deleter(std::move(deleter))
+ : m_ptr(reinterpret_cast<T*>(allocation)), m_deleter(deleter)
 {}
 
+
+
+template<typename T>
+unique_ptr<T>& unique_ptr<T>::operator=(unique_ptr&& rhs) noexcept
+{
+ if(this != &rhs)
+ {
+ reset(rhs.m_ptr);
+ m_deleter = std::move(rhs.m_deleter);
+ }
+ return *this;
+}
+
+
 template<typename T>
 unique_ptr<T>::unique_ptr(unique_ptr&& rhs) noexcept
 {
- m_ptr = rhs.m_ptr;
- m_deleter = std::move(rhs.m_deleter);
+ *this = std::move(rhs);
 }
 
 template<typename T>
 unique_ptr<T>::~unique_ptr() noexcept
 {
- if(m_deleter)
- {
- m_deleter(m_ptr);
- }
+ reset();
 }
 
 /// Dereference the stored pointer.
@@ -70,7 +80,7 @@ T* unique_ptr<T>::release() noexcept
 template<typename T>
 void unique_ptr<T>::reset(T* ptr) noexcept
 {
- if(m_ptr)
+ if(m_ptr && m_deleter)
  {
  m_deleter(m_ptr);
  }

iceoryx_utils/test/moduletests/test_cxx_unique_ptr.cpp

@@ -88,3 +88,28 @@ TEST_F(UniquePtrTest, DeleterIsCalledWhenPtrGoesOutOfScope)
 
  ASSERT_EQ(true, deleterCalled);
 }
+
+TEST_F(UniquePtrTest, DeleterIsProperlySet)
+{
+
+}
+
+TEST_F(UniquePtrTest, DeleterNotCalledOnReleasedPointers)
+{
+
+}
+
+TEST_F(UniquePtrTest, DeleterNotCalledOnNullptrs)
+{
+
+}
+
+TEST_F(UniquePtrTest, CanResetToNullptr)
+{
+
+}
+
+TEST_F(UniquePtrTest, CanResetToAnExistingRawPtr)
+{
+
+}