[#490] Address review findings

eclipse-iceoryx · Nov 1, 2024 · 0e22135 · 0e22135
1 parent 7ce0aca
commit 0e22135
Show file tree

Hide file tree

Showing 4 changed files with 76 additions and 56 deletions.
diff --git a/iceoryx2-ffi/cxx/include/iox/slice.hpp b/iceoryx2-ffi/cxx/include/iox/slice.hpp
@@ -26,61 +26,81 @@ class Slice {
     using ConstIterator = const T*;
     using ValueType = T;
 
-    Slice(const void* data, uint64_t number_of_elements)
-        : m_data { static_cast<T*>(const_cast<void*>(data)) }
-        , m_number_of_elements { number_of_elements } {
-    }
-
-    auto number_of_elements() const -> uint64_t {
-        return m_number_of_elements;
-    }
-    auto operator[](const uint64_t n) const -> const T& {
-        IOX_ASSERT(n < m_number_of_elements, "Index out of bounds");
-        return *(m_data + n);
-    }
-
-    auto operator[](const uint64_t n) -> T& {
-        IOX_ASSERT(n < m_number_of_elements, "Index out of bounds");
-        return *(m_data + n);
-    }
-
-    auto begin() -> Iterator {
-        return m_data;
-    }
-
-    auto begin() const -> ConstIterator {
-        return m_data;
-    }
-
-    auto end() -> Iterator {
-        if constexpr (std::is_same_v<T, void>) {
-            return reinterpret_cast<T*>(reinterpret_cast<uint8_t*>(m_data) + m_number_of_elements);
-        } else {
-            return m_data + m_number_of_elements;
-        }
-    }
-
-    auto end() const -> ConstIterator {
-        if constexpr (std::is_same_v<T, void>) {
-            return reinterpret_cast<const T*>(reinterpret_cast<const uint8_t*>(m_data) + m_number_of_elements);
-        } else {
-            return m_data + m_number_of_elements;
-        }
-    }
-
-    auto data() -> T* {
-        return m_data;
-    };
-
-    auto data() const -> const T* {
-        return m_data;
-    };
+    Slice(const T* data, uint64_t number_of_elements);
+
+    auto number_of_elements() const -> uint64_t;
+    auto operator[](uint64_t n) const -> const T&;
+    auto operator[](uint64_t n) -> T&;
+
+    auto begin() -> Iterator;
+    auto begin() const -> ConstIterator;
+    auto end() -> Iterator;
+    auto end() const -> ConstIterator;
+
+    auto data() -> T*;
+    auto data() const -> const T*;
 
   private:
     T* m_data;
     uint64_t m_number_of_elements;
 };
 
+template <typename T>
+Slice<T>::Slice(const T* data, uint64_t number_of_elements)
+    // NOLINTNEXTLINE(cppcoreguidelines-pro-type-const-cast) constness protected by const class specification
+    : m_data { const_cast<T*>(data) }
+    , m_number_of_elements { number_of_elements } {
+}
+
+template <typename T>
+auto Slice<T>::number_of_elements() const -> uint64_t {
+    return m_number_of_elements;
+}
+
+template <typename T>
+auto Slice<T>::operator[](const uint64_t n) const -> const T& {
+    IOX_ASSERT(n < m_number_of_elements, "Index out of bounds");
+    return *(m_data + n);
+}
+
+template <typename T>
+auto Slice<T>::operator[](const uint64_t n) -> T& {
+    IOX_ASSERT(n < m_number_of_elements, "Index out of bounds");
+    return *(m_data + n);
+}
+
+template <typename T>
+auto Slice<T>::begin() -> Iterator {
+    return m_data;
+}
+
+template <typename T>
+auto Slice<T>::begin() const -> ConstIterator {
+    return m_data;
+}
+
+template <typename T>
+auto Slice<T>::end() -> Iterator {
+    static_assert(!std::is_same_v<T, void>, "Slice<void> is not allowed");
+    return m_data + m_number_of_elements;
+}
+
+template <typename T>
+auto Slice<T>::end() const -> ConstIterator {
+    static_assert(!std::is_same_v<T, void>, "Slice<void> is not allowed");
+    return m_data + m_number_of_elements;
+}
+
+template <typename T>
+auto Slice<T>::data() -> T* {
+    return m_data;
+}
+
+template <typename T>
+auto Slice<T>::data() const -> const T* {
+    return m_data;
+}
+
 template <typename>
 struct IsSlice {
     static constexpr bool VALUE = false;

diff --git a/iceoryx2-ffi/cxx/include/iox2/sample.hpp b/iceoryx2-ffi/cxx/include/iox2/sample.hpp
@@ -147,7 +147,7 @@ inline auto Sample<S, Payload, UserHeader>::payload_slice() const -> Payload {
 
     size_t number_of_elements = number_of_bytes / sizeof(typename Payload::ValueType);
 
-    return Payload(ptr, number_of_elements);
+    return Payload(static_cast<const typename Payload::ValueType*>(ptr), number_of_elements);
 }
 
 template <ServiceType S, typename Payload, typename UserHeader>

diff --git a/iceoryx2-ffi/cxx/include/iox2/sample_mut.hpp b/iceoryx2-ffi/cxx/include/iox2/sample_mut.hpp
@@ -227,7 +227,7 @@ inline auto SampleMut<S, Payload, UserHeader>::payload_slice() const -> const Pa
 
     size_t number_of_elements = number_of_bytes / sizeof(typename Payload::ValueType);
 
-    return Payload(ptr, number_of_elements);
+    return Payload(static_cast<typename Payload::ValueType*>(const_cast<void*>(ptr)), number_of_elements);
 }
 
 template <ServiceType S, typename Payload, typename UserHeader>
@@ -240,7 +240,7 @@ inline auto SampleMut<S, Payload, UserHeader>::payload_slice_mut() -> Payload {
 
     size_t number_of_elements = number_of_bytes / sizeof(typename Payload::ValueType);
 
-    return Payload(ptr, number_of_elements);
+    return Payload(static_cast<typename Payload::ValueType*>(const_cast<void*>(ptr)), number_of_elements);
 }
 
 template <ServiceType S, typename Payload, typename UserHeader>

diff --git a/iceoryx2-ffi/ffi/src/api/sample_mut.rs b/iceoryx2-ffi/ffi/src/api/sample_mut.rs
@@ -157,7 +157,7 @@ pub unsafe extern "C" fn iox2_sample_mut_move(
 ///
 /// # Safety
 ///
-/// * `handle` obtained by [`iox2_publisher_loan()`](crate::iox2_publisher_loan())
+/// * `handle` obtained by [`iox2_publisher_loan_slice_uninit()`](crate::iox2_publisher_loan_slice_uninit())
 /// * `header_ptr` a valid, non-null pointer pointing to a [`*const c_void`] pointer.
 #[no_mangle]
 pub unsafe extern "C" fn iox2_sample_mut_user_header(
@@ -181,7 +181,7 @@ pub unsafe extern "C" fn iox2_sample_mut_user_header(
 ///
 /// # Safety
 ///
-/// * `handle` obtained by [`iox2_publisher_loan()`](crate::iox2_publisher_loan())
+/// * `handle` obtained by [`iox2_publisher_loan_slice_uninit()`](crate::iox2_publisher_loan_slice_uninit())
 /// * `header_struct_ptr` - Must be either a NULL pointer or a pointer to a valid
 ///     [`iox2_publish_subscribe_header_t`]. If it is a NULL pointer, the storage will be allocated on the heap.
 /// * `header_handle_ptr` valid pointer to a [`iox2_publish_subscribe_header_h`].
@@ -218,7 +218,7 @@ pub unsafe extern "C" fn iox2_sample_mut_header(
 ///
 /// # Safety
 ///
-/// * `handle` obtained by [`iox2_publisher_loan()`](crate::iox2_publisher_loan())
+/// * `handle` obtained by [`iox2_publisher_loan_slice_uninit()`](crate::iox2_publisher_loan_slice_uninit())
 /// * `header_ptr` a valid, non-null pointer pointing to a [`*const c_void`] pointer.
 #[no_mangle]
 pub unsafe extern "C" fn iox2_sample_mut_user_header_mut(
@@ -242,7 +242,7 @@ pub unsafe extern "C" fn iox2_sample_mut_user_header_mut(
 ///
 /// # Safety
 ///
-/// * `handle` obtained by [`iox2_publisher_loan()`](crate::iox2_publisher_loan())
+/// * `handle` obtained by [`iox2_publisher_loan_slice_uninit()`](crate::iox2_publisher_loan_slice_uninit())
 /// * `payload_ptr` a valid, non-null pointer pointing to a [`*const c_void`] pointer.
 /// * `payload_len` (optional) either a null poitner or a valid pointer pointing to a [`c_size_t`].
 #[no_mangle]