iox-eclipse-iceoryx#2157 Add tests and incorporate suggestions from r…

…eview

doc/design/chunk_header.md

@@ -43,26 +43,26 @@ Framing with terminology
 ```
 class ChunkHeader
 {
-    uint32_t chunkSize;
+    uint32_t userHeaderSize{0U};
     uint8_t chunkHeaderVersion;
     uint8_t reserved{0};
     uint16_t userHeaderId;
     popo::UniquePortId originId; // underlying type = uint64_t
     uint64_t sequenceNumber;
-    uint32_t userHeaderSize{0U};
+    uint64_t chunkSize;
     uint32_t userPayloadSize{0U};
     uint32_t userPayloadAlignment{1U};
     UserPayloadOffset_t userPayloadOffset; // alias to uint32_t
 };
 ```
 
-- **chunkSize** is the size of the whole chunk
+- **userHeaderSize** is the size of the chunk occupied by the user-header
 - **chunkHeaderVersion** is used to detect incompatibilities for record&replay functionality
 - **reserved** is currently not used and set to `0`
 - **userHeaderId** is currently not used and set to `NO_USER_HEADER`
 - **originId** is the unique identifier of the publisher the chunk was sent from
 - **sequenceNumber** is a serial number for the sent chunks
-- **userHeaderSize** is the size of the chunk occupied by the user-header
+- **chunkSize** is the size of the whole chunk
 - **userPayloadSize** is the size of the chunk occupied by the user-payload
 - **userPayloadAlignment** is the alignment of the chunk occupied by the user-payload
 - **userPayloadOffset** is the offset of the user-payload relative to the begin of the chunk

doc/website/release-notes/iceoryx-unreleased.md

@@ -1293,30 +1293,8 @@
     iox::concurrent::MpmcLockFreeQueue q;
     ```
 
-59. Payload Size for Memory Chunks is now `uin64_t`. Therefore the `ChunkHeader` layout changes:
+59. Payload Size for Memory Chunks is now `uin64_t`.
+    Hence the `ChunkHeader` (iceoryx_posh/mepoo/chunk_header.hpp) layout changes
+    and `m_chunkHeaderVersion` is getting increased.
+    Moreover many functions' signatures are also affected by this change.
 
-    ```cpp
-    // before
-    uint32_t m_chunkSize;
-    uint8_t m_chunkHeaderVersion;
-    uint8_t m_reserved;
-    uint16_t m_userHeaderId;
-    popo::UniquePortId m_originId;
-    uint64_t m_sequenceNumber;
-    uint32_t m_userHeaderSize;
-    uint64_t m_userPayloadSize;
-    uint32_t m_userPayloadAlignment;
-    UserPayloadOffset_t m_userPayloadOffset;
-
-    // after
-    uint32_t m_userHeaderSize;
-    uint8_t m_chunkHeaderVersion;
-    uint8_t m_reserved;
-    uint16_t m_userHeaderId;
-    popo::UniquePortId m_originId;
-    uint64_t m_sequenceNumber;
-    uint64_t m_chunkSize;
-    uint64_t m_userPayloadSize;
-    uint32_t m_userPayloadAlignment;
-    UserPayloadOffset_t m_userPayloadOffset;
-    ```

iceoryx_posh/include/iceoryx_posh/mepoo/chunk_settings.hpp

@@ -35,6 +35,7 @@ class ChunkSettings
         ALIGNMENT_NOT_POWER_OF_TWO,
         USER_HEADER_ALIGNMENT_EXCEEDS_CHUNK_HEADER_ALIGNMENT,
         USER_HEADER_SIZE_NOT_MULTIPLE_OF_ITS_ALIGNMENT,
+        REQUIRED_CHUNK_SIZE_EXCEEDS_MAX_CHUNK_SIZE,
     };
 
     /// @brief constructs and initializes a ChunkSettings
@@ -76,9 +77,9 @@ class ChunkSettings
                   const uint32_t userHeaderAlignment,
                   const uint64_t requiredChunkSize) noexcept;
 
-    static uint64_t calculateRequiredChunkSize(const uint64_t userPayloadSize,
-                                               const uint32_t userPayloadAlignment,
-                                               const uint32_t userHeaderSize) noexcept;
+    static expected<uint64_t, ChunkSettings::Error> calculateRequiredChunkSize(const uint64_t userPayloadSize,
+                                                                               const uint32_t userPayloadAlignment,
+                                                                               const uint32_t userHeaderSize) noexcept;
 
   private:
     uint64_t m_userPayloadSize{0U};

iceoryx_posh/include/iceoryx_posh/mepoo/mepoo_config.hpp

@@ -35,9 +35,9 @@ struct MePooConfig
     struct Entry
     {
         /// @brief set the size and count of memory chunks
-        Entry(uint64_t f_size, uint32_t f_chunkCount) noexcept
-            : m_size(f_size)
-            , m_chunkCount(f_chunkCount)
+        Entry(uint64_t size, uint32_t chunkCount) noexcept
+            : m_size(size)
+            , m_chunkCount(chunkCount)
         {
         }
         uint64_t m_size{0};
@@ -56,7 +56,7 @@ struct MePooConfig
 
     /// @brief Function for adding new entry
     /// @param[in] Entry structure of mempool configuration
-    void addMemPool(Entry f_entry) noexcept;
+    void addMemPool(Entry entry) noexcept;
 
     /// @brief Function for creating default memory pools
     MePooConfig& setDefaults() noexcept;

iceoryx_posh/include/iceoryx_posh/roudi/introspection_types.hpp

@@ -95,7 +95,7 @@ const capro::ServiceDescription
 struct PortThroughputData
 {
     uint64_t m_publisherPortID{0};
-    uint32_t m_sampleSize{0};
+    uint64_t m_sampleSize{0};
     uint64_t m_chunkSize{0};
     double m_chunksPerMinute{0};
     uint64_t m_lastSendIntervalInNanoseconds{0};

iceoryx_posh/roudi_env/include/iceoryx_posh/roudi_env/minimal_roudi_config.hpp

@@ -29,7 +29,7 @@ namespace roudi_env
 class MinimalRouDiConfigBuilder
 {
     /// @brief Set the payload chunk size. Default = 128
-    IOX_BUILDER_PARAMETER(uint32_t, payloadChunkSize, 128)
+    IOX_BUILDER_PARAMETER(uint64_t, payloadChunkSize, 128)
 
     /// @brief Set the payload chunk count. Default = 10
     IOX_BUILDER_PARAMETER(uint32_t, payloadChunkCount, 10)

iceoryx_posh/source/mepoo/chunk_settings.cpp

@@ -63,33 +63,50 @@ expected<ChunkSettings, ChunkSettings::Error> ChunkSettings::create(const uint64
         return err(ChunkSettings::Error::USER_HEADER_SIZE_NOT_MULTIPLE_OF_ITS_ALIGNMENT);
     }
 
-    uint64_t requiredChunkSize =
-        calculateRequiredChunkSize(userPayloadSize, adjustedUserPayloadAlignment, userHeaderSize);
+    auto expectChunkSize = calculateRequiredChunkSize(userPayloadSize, adjustedUserPayloadAlignment, userHeaderSize);
+    if (expectChunkSize.has_error())
+    {
+        return err(expectChunkSize.get_error());
+    }
+    uint64_t requiredChunkSize = expectChunkSize.value();
+
 
     return ok(ChunkSettings{
         userPayloadSize, adjustedUserPayloadAlignment, userHeaderSize, adjustedUserHeaderAlignment, requiredChunkSize});
 }
-uint64_t ChunkSettings::calculateRequiredChunkSize(const uint64_t userPayloadSize,
-                                                   const uint32_t userPayloadAlignment,
-                                                   const uint32_t userHeaderSize) noexcept
+
+expected<uint64_t, ChunkSettings::Error> ChunkSettings::calculateRequiredChunkSize(
+    const uint64_t userPayloadSize, const uint32_t userPayloadAlignment, const uint32_t userHeaderSize) noexcept
 {
     // have a look at »Required Chunk Size Calculation« in chunk_header.md for more details regarding the calculation
     if (userHeaderSize == 0)
     {
         // the most simple case with no user-header and the user-payload adjacent to the ChunkHeader
         if (userPayloadAlignment <= alignof(mepoo::ChunkHeader))
         {
+            if (userPayloadSize > std::numeric_limits<uint64_t>::max() - sizeof(ChunkHeader))
+            {
+                return err(ChunkSettings::Error::REQUIRED_CHUNK_SIZE_EXCEEDS_MAX_CHUNK_SIZE);
+            }
+
             uint64_t requiredChunkSize = sizeof(ChunkHeader) + userPayloadSize;
 
-            return requiredChunkSize;
+            return ok(requiredChunkSize);
         }
 
         // the second most simple case with no user-header but the user-payload alignment
         // exceeds the ChunkHeader alignment and is therefore not necessarily adjacent
         uint64_t preUserPayloadAlignmentOverhang = sizeof(ChunkHeader) - alignof(ChunkHeader);
+
+        if (userPayloadSize
+            > std::numeric_limits<uint64_t>::max() - preUserPayloadAlignmentOverhang - userPayloadAlignment)
+        {
+            return err(ChunkSettings::Error::REQUIRED_CHUNK_SIZE_EXCEEDS_MAX_CHUNK_SIZE);
+        }
+
         uint64_t requiredChunkSize = preUserPayloadAlignmentOverhang + userPayloadAlignment + userPayloadSize;
 
-        return requiredChunkSize;
+        return ok(requiredChunkSize);
     }
 
     // the most complex case with a user-header
@@ -98,9 +115,15 @@ uint64_t ChunkSettings::calculateRequiredChunkSize(const uint64_t userPayloadSiz
     uint64_t headerSize = sizeof(ChunkHeader) + userHeaderSize;
     uint64_t preUserPayloadAlignmentOverhang = align(headerSize, ALIGNMENT_OF_USER_PAYLOAD_OFFSET_T);
     uint64_t maxPadding = algorithm::maxVal(SIZE_OF_USER_PAYLOAD_OFFSET_T, static_cast<uint64_t>(userPayloadAlignment));
+
+    if (userPayloadSize > std::numeric_limits<uint64_t>::max() - preUserPayloadAlignmentOverhang - maxPadding)
+    {
+        return err(ChunkSettings::Error::REQUIRED_CHUNK_SIZE_EXCEEDS_MAX_CHUNK_SIZE);
+    }
+
     uint64_t requiredChunkSize = preUserPayloadAlignmentOverhang + maxPadding + userPayloadSize;
 
-    return requiredChunkSize;
+    return ok(requiredChunkSize);
 }
 
 uint64_t ChunkSettings::requiredChunkSize() const noexcept

iceoryx_posh/source/mepoo/mem_pool.cpp

@@ -49,6 +49,7 @@ MemPool::MemPool(const greater_or_equal<uint64_t, CHUNK_MEMORY_ALIGNMENT> chunkS
 {
     if (isMultipleOfAlignment(chunkSize))
     {
+        IOX_EXPECTS(m_chunkSize <= std::numeric_limits<uint64_t>::max() / m_numberOfChunks);
         auto allocationResult = chunkMemoryAllocator.allocate(static_cast<uint64_t>(m_numberOfChunks) * m_chunkSize,
                                                               CHUNK_MEMORY_ALIGNMENT);
         IOX_EXPECTS(allocationResult.has_value());
@@ -63,9 +64,8 @@ MemPool::MemPool(const greater_or_equal<uint64_t, CHUNK_MEMORY_ALIGNMENT> chunkS
     else
     {
         IOX_LOG(FATAL,
-                "Chunk size must be multiple of '" << CHUNK_MEMORY_ALIGNMENT << "'! Requested size is "
-                                                   << static_cast<uint64_t>(chunkSize) << " for "
-                                                   << static_cast<uint32_t>(numberOfChunks) << " chunks!");
+                "Chunk size must be multiple of '" << CHUNK_MEMORY_ALIGNMENT << "'! Requested size is " << chunkSize
+                                                   << " for " << numberOfChunks << " chunks!");
         errorHandler(PoshError::MEPOO__MEMPOOL_CHUNKSIZE_MUST_BE_MULTIPLE_OF_CHUNK_MEMORY_ALIGNMENT);
     }
 }

iceoryx_posh/source/mepoo/memory_manager.cpp

@@ -58,8 +58,8 @@ void MemoryManager::addMemPool(BumpAllocator& managementAllocator,
                 return log;
             } << "These mempools must be added in an increasing chunk size ordering. The newly added  MemPool [ "
                  "ChunkSize = "
-              << adjustedChunkSize << ", ChunkPayloadSize = " << static_cast<uint64_t>(chunkPayloadSize)
-              << ", ChunkCount = " << static_cast<uint32_t>(numberOfChunks) << "] breaks that requirement!");
+              << adjustedChunkSize << ", ChunkPayloadSize = " << chunkPayloadSize << ", ChunkCount = " << numberOfChunks
+              << "] breaks that requirement!");
         errorHandler(iox::PoshError::MEPOO__MEMPOOL_CONFIG_MUST_BE_ORDERED_BY_INCREASING_SIZE);
     }
 

iceoryx_posh/source/mepoo/mepoo_config.cpp

@@ -28,11 +28,11 @@ const MePooConfig::MePooConfigContainerType* MePooConfig::getMemPoolConfig() con
     return &m_mempoolConfig;
 }
 
-void MePooConfig::addMemPool(MePooConfig::Entry f_entry) noexcept
+void MePooConfig::addMemPool(MePooConfig::Entry entry) noexcept
 {
     if (m_mempoolConfig.size() < m_mempoolConfig.capacity())
     {
-        m_mempoolConfig.push_back(f_entry);
+        m_mempoolConfig.push_back(entry);
     }
     else
     {