From 898afdbfdef3c4bd7e02173f54236345d64fa540 Mon Sep 17 00:00:00 2001 From: Christian Eltzschig Date: Mon, 24 Jan 2022 17:28:01 +0100 Subject: [PATCH] iox-#1036 Implement IOX_BUILDER_PARAMETER to create builder patterns more easily and implemented MemoryMapBuilder with that Signed-off-by: Christian Eltzschig --- .../include/iceoryx_hoofs/cxx/helplets.hpp | 44 +++++++++++++++++++ .../shared_memory_object/memory_map.hpp | 30 +++++++------ .../posix_wrapper/shared_memory_object.cpp | 14 +++--- .../shared_memory_object/memory_map.cpp | 23 +++++----- 4 files changed, 80 insertions(+), 31 deletions(-) diff --git a/iceoryx_hoofs/include/iceoryx_hoofs/cxx/helplets.hpp b/iceoryx_hoofs/include/iceoryx_hoofs/cxx/helplets.hpp index 56876a2466..d32d45b124 100644 --- a/iceoryx_hoofs/include/iceoryx_hoofs/cxx/helplets.hpp +++ b/iceoryx_hoofs/include/iceoryx_hoofs/cxx/helplets.hpp @@ -335,6 +335,50 @@ constexpr T from(const F value) noexcept; template constexpr T into(const F value) noexcept; +/// @brief Macro which generates a setter method useful for a builder pattern. +/// @param[in] type the data type of the parameter +/// @param[in] name the name of the parameter +/// @param[in] defaultValue the default value of the parameter +/// @code +/// class MyBuilder { +/// IOX_BUILDER_PARAMETER(TypeA, NameB, ValueC) +/// // START generates the following code +/// private: +/// TypeA m_NameB = ValueC; +/// +/// public: +/// decltype(auto) NameB(TypeA const& value) && +/// { +/// m_NameB = value; +/// return std::move(*this); +/// } +/// +/// decltype(auto) NameB(TypeA&& value) && +/// { +/// m_NameB = std::move(value); +/// return std::move(*this); +/// } +/// // END +/// }; +/// @endcode +#define IOX_BUILDER_PARAMETER(type, name, defaultValue) \ + private: \ + type m_##name = defaultValue; \ + \ + public: \ + decltype(auto) name(type const& value)&& \ + { \ + m_##name = value; \ + return std::move(*this); \ + } \ + \ + decltype(auto) name(type&& value)&& \ + { \ + m_##name = std::move(value); \ + return std::move(*this); \ + } + + } // namespace cxx } // namespace iox diff --git a/iceoryx_hoofs/include/iceoryx_hoofs/internal/posix_wrapper/shared_memory_object/memory_map.hpp b/iceoryx_hoofs/include/iceoryx_hoofs/internal/posix_wrapper/shared_memory_object/memory_map.hpp index e1d0beb6eb..6a7fb2d123 100644 --- a/iceoryx_hoofs/include/iceoryx_hoofs/internal/posix_wrapper/shared_memory_object/memory_map.hpp +++ b/iceoryx_hoofs/include/iceoryx_hoofs/internal/posix_wrapper/shared_memory_object/memory_map.hpp @@ -17,6 +17,7 @@ #ifndef IOX_HOOFS_POSIX_WRAPPER_SHARED_MEMORY_OBJECT_MEMORY_MAP_HPP #define IOX_HOOFS_POSIX_WRAPPER_SHARED_MEMORY_OBJECT_MEMORY_MAP_HPP +#include "iceoryx_hoofs/cxx/helplets.hpp" #include "iceoryx_hoofs/cxx/optional.hpp" #include "iceoryx_hoofs/design_pattern/creation.hpp" #include "iceoryx_hoofs/internal/posix_wrapper/shared_memory_object/shared_memory.hpp" @@ -62,30 +63,37 @@ enum class MemoryMapFlags : int32_t PRIVATE_CHANGES_AND_FORCE_BASE_ADDRESS_HINT = MAP_PRIVATE | MAP_FIXED, }; -/// @brief The configuration of a MemoryMap object -struct MemoryMapConfig +class MemoryMap; +/// @brief The builder of a MemoryMap object +class MemoryMapBuilder { /// @brief The base address suggestion to which the memory should be mapped. But /// there is not guarantee that it is really mapped at this position. /// One has to verify with .getBaseAddress if the hint was accepted. /// Setting it to nullptr means no suggestion - const void* baseAddressHint = nullptr; + IOX_BUILDER_PARAMETER(const void*, baseAddressHint, nullptr) /// @brief The length of the memory which should be mapped - uint64_t length = 0U; + IOX_BUILDER_PARAMETER(uint64_t, length, 0U) /// @brief The file descriptor which should be mapped into process space - int32_t fileDescriptor = 0; + IOX_BUILDER_PARAMETER(int32_t, fileDescriptor, 0) /// @brief Defines if the memory should be mapped read only or with write access. /// A read only memory section will cause a segmentation fault when be written to. - AccessMode accessMode = AccessMode::READ_WRITE; + IOX_BUILDER_PARAMETER(AccessMode, accessMode, AccessMode::READ_WRITE) /// @brief Sets the flags defining how the mapped data should be handled - MemoryMapFlags flags = MemoryMapFlags::SHARE_CHANGES; + IOX_BUILDER_PARAMETER(MemoryMapFlags, flags, MemoryMapFlags::SHARE_CHANGES) /// @brief Offset of the memory location - off_t offset = 0; + IOX_BUILDER_PARAMETER(off_t, offset, 0) + + public: + /// @brief creates a valid MemoryMap object. If the construction failed the expected + /// contains an enum value describing the error. + /// @return expected containing MemoryMap on success otherwise MemoryMapError + cxx::expected create() noexcept; }; /// @brief C++ abstraction of mmap and munmap. When a MemoryMap object is @@ -117,11 +125,7 @@ class MemoryMap /// @brief returns the base address, if the object was moved it returns nullptr void* getBaseAddress() noexcept; - /// @brief creates a valid MemoryMap object. If the construction failed the expected - /// contains an enum value describing the error. - /// @param[in] config the mmap configuration - /// @return expected containing MemoryMap on success otherwise MemoryMapError - static cxx::expected create(const MemoryMapConfig& config) noexcept; + friend class MemoryMapBuilder; private: MemoryMap(void* const baseAddress, const uint64_t length) noexcept; diff --git a/iceoryx_hoofs/source/posix_wrapper/shared_memory_object.cpp b/iceoryx_hoofs/source/posix_wrapper/shared_memory_object.cpp index 3f5691356c..057dd36acd 100644 --- a/iceoryx_hoofs/source/posix_wrapper/shared_memory_object.cpp +++ b/iceoryx_hoofs/source/posix_wrapper/shared_memory_object.cpp @@ -68,12 +68,14 @@ SharedMemoryObject::SharedMemoryObject(const SharedMemory::Name_t& name, if (m_isInitialized) { - MemoryMap::create({baseAddressHint, - m_memorySizeInBytes, - m_sharedMemory->getHandle(), - accessMode, - MemoryMapFlags::SHARE_CHANGES, - 0}) + MemoryMapBuilder() + .baseAddressHint(baseAddressHint) + .length(memorySizeInBytes) + .fileDescriptor(m_sharedMemory->getHandle()) + .accessMode(accessMode) + .flags(MemoryMapFlags::SHARE_CHANGES) + .offset(0) + .create() .and_then([this](auto& memoryMap) { m_memoryMap.emplace(std::move(memoryMap)); }) .or_else([this](auto) { std::cerr << "Failed to map created shared memory into process!" << std::endl; diff --git a/iceoryx_hoofs/source/posix_wrapper/shared_memory_object/memory_map.cpp b/iceoryx_hoofs/source/posix_wrapper/shared_memory_object/memory_map.cpp index 43fbf724c2..92692b223a 100644 --- a/iceoryx_hoofs/source/posix_wrapper/shared_memory_object/memory_map.cpp +++ b/iceoryx_hoofs/source/posix_wrapper/shared_memory_object/memory_map.cpp @@ -24,10 +24,10 @@ namespace iox { namespace posix { -cxx::expected MemoryMap::create(const MemoryMapConfig& config) noexcept +cxx::expected MemoryMapBuilder::create() noexcept { int32_t l_memoryProtection{PROT_NONE}; - switch (config.accessMode) + switch (m_accessMode) { case AccessMode::READ_ONLY: l_memoryProtection = PROT_READ; @@ -39,29 +39,28 @@ cxx::expected MemoryMap::create(const MemoryMapConfig // PRQA S 3066 1 # incompatibility with POSIX definition of mmap // NOLINTNEXTLINE(cppcoreguidelines-pro-type-const-cast) low-level memory management - auto result = posixCall(mmap)(const_cast(config.baseAddressHint), - config.length, + auto result = posixCall(mmap)(const_cast(m_baseAddressHint), + m_length, l_memoryProtection, - static_cast(config.flags), - config.fileDescriptor, - config.offset) + static_cast(m_flags), + m_fileDescriptor, + m_offset) // NOLINTNEXTLINE(cppcoreguidelines-pro-type-cstyle-cast, performance-no-int-to-ptr) .failureReturnValue(reinterpret_cast(MAP_FAILED)) .evaluate(); if (result) { - return cxx::success(MemoryMap(result.value().value, config.length)); + return cxx::success(MemoryMap(result.value().value, m_length)); } constexpr uint64_t FLAGS_BIT_SIZE = 32U; auto flags = std::cerr.flags(); std::cerr << "Unable to map memory with the following properties [ baseAddressHint = " << std::hex - << config.baseAddressHint << ", length = " << std::dec << config.length - << ", fileDescriptor = " << config.fileDescriptor - << ", access mode = " << ACCESS_MODE_STRING[static_cast(config.accessMode)] + << m_baseAddressHint << ", length = " << std::dec << m_length << ", fileDescriptor = " << m_fileDescriptor + << ", access mode = " << ACCESS_MODE_STRING[static_cast(m_accessMode)] << ", flags = " << std::bitset(static_cast(flags)) << ", offset = " << std::hex - << config.offset << std::dec << " ]" << std::endl; + << m_offset << std::dec << " ]" << std::endl; std::cerr.setf(flags); return cxx::error(MemoryMap::errnoToEnum(result.get_error().errnum)); }