Skip to content

Commit

Permalink
iox-eclipse-iceoryx#1036 Implement IOX_BUILDER_PARAMETER to create bu…
Browse files Browse the repository at this point in the history
…ilder patterns more easily and implemented MemoryMapBuilder with that

Signed-off-by: Christian Eltzschig <me@elchris.org>
  • Loading branch information
elfenpiff committed Jan 24, 2022
1 parent 4f47265 commit 898afdb
Show file tree
Hide file tree
Showing 4 changed files with 80 additions and 31 deletions.
44 changes: 44 additions & 0 deletions iceoryx_hoofs/include/iceoryx_hoofs/cxx/helplets.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -335,6 +335,50 @@ constexpr T from(const F value) noexcept;
template <typename T, typename F>
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

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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<MemoryMap, MemoryMapError> create() noexcept;
};

/// @brief C++ abstraction of mmap and munmap. When a MemoryMap object is
Expand Down Expand Up @@ -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<MemoryMap, MemoryMapError> create(const MemoryMapConfig& config) noexcept;
friend class MemoryMapBuilder;

private:
MemoryMap(void* const baseAddress, const uint64_t length) noexcept;
Expand Down
14 changes: 8 additions & 6 deletions iceoryx_hoofs/source/posix_wrapper/shared_memory_object.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,10 +24,10 @@ namespace iox
{
namespace posix
{
cxx::expected<MemoryMap, MemoryMapError> MemoryMap::create(const MemoryMapConfig& config) noexcept
cxx::expected<MemoryMap, MemoryMapError> MemoryMapBuilder::create() noexcept
{
int32_t l_memoryProtection{PROT_NONE};
switch (config.accessMode)
switch (m_accessMode)
{
case AccessMode::READ_ONLY:
l_memoryProtection = PROT_READ;
Expand All @@ -39,29 +39,28 @@ cxx::expected<MemoryMap, MemoryMapError> 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<void*>(config.baseAddressHint),
config.length,
auto result = posixCall(mmap)(const_cast<void*>(m_baseAddressHint),
m_length,
l_memoryProtection,
static_cast<int32_t>(config.flags),
config.fileDescriptor,
config.offset)
static_cast<int32_t>(m_flags),
m_fileDescriptor,
m_offset)
// NOLINTNEXTLINE(cppcoreguidelines-pro-type-cstyle-cast, performance-no-int-to-ptr)
.failureReturnValue(reinterpret_cast<void*>(MAP_FAILED))
.evaluate();

if (result)
{
return cxx::success<MemoryMap>(MemoryMap(result.value().value, config.length));
return cxx::success<MemoryMap>(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<uint64_t>(config.accessMode)]
<< m_baseAddressHint << ", length = " << std::dec << m_length << ", fileDescriptor = " << m_fileDescriptor
<< ", access mode = " << ACCESS_MODE_STRING[static_cast<uint64_t>(m_accessMode)]
<< ", flags = " << std::bitset<FLAGS_BIT_SIZE>(static_cast<uint32_t>(flags)) << ", offset = " << std::hex
<< config.offset << std::dec << " ]" << std::endl;
<< m_offset << std::dec << " ]" << std::endl;
std::cerr.setf(flags);
return cxx::error<MemoryMapError>(MemoryMap::errnoToEnum(result.get_error().errnum));
}
Expand Down

0 comments on commit 898afdb

Please sign in to comment.