Iox #1036 in place creation for shared memory

CHANGELOG.md

@@ -68,6 +68,8 @@
 - Fix error handling of `TypedUniqueId` and refactor it to `UniquePortId` [\#861](https://github.com/eclipse-iceoryx/iceoryx/issues/861)
 - Updating Codecov API and enforce CMake version 3.16 for building iceoryx [\#774](https://github.com/eclipse-iceoryx/iceoryx/issues/774) and [\#1031](https://github.com/eclipse-iceoryx/iceoryx/issues/1031)
 - Remove `InvalidIdString` and `isValid()` from `ServiceDescription`, replace Wildcard string with `iox::cxx::nullopt` [\#415](https://github.com/eclipse-iceoryx/iceoryx/issues/415)
+- Remove creation pattern from `SharedMemory` and replace it with `SharedMemoryBuilder` [\#1036](https://github.com/eclipse-iceoryx/iceoryx/issues/1036)
+- Remove the leading slash requirement from the name of a shared memory in `SharedMemory` and `SharedMemoryObject` [\#439](https://github.com/eclipse-iceoryx/iceoryx/issues/439)
 
 **New API features:**
 

iceoryx_hoofs/include/iceoryx_hoofs/cxx/helplets.hpp

@@ -356,7 +356,7 @@ constexpr T into(const F value) noexcept;
     }                                                                                                                  \
                                                                                                                        \
   private:                                                                                                             \
-    type m_##name = defaultValue;
+    type m_##name{defaultValue};
 
 } // namespace cxx
 } // namespace iox

iceoryx_hoofs/include/iceoryx_hoofs/internal/posix_wrapper/shared_memory_object.hpp

@@ -17,6 +17,7 @@
 #ifndef IOX_HOOFS_POSIX_WRAPPER_SHARED_MEMORY_OBJECT_HPP
 #define IOX_HOOFS_POSIX_WRAPPER_SHARED_MEMORY_OBJECT_HPP
 
+#include "iceoryx_hoofs/cxx/filesystem.hpp"
 #include "iceoryx_hoofs/cxx/optional.hpp"
 #include "iceoryx_hoofs/design_pattern/creation.hpp"
 #include "iceoryx_hoofs/internal/posix_wrapper/shared_memory_object/allocator.hpp"
@@ -67,7 +68,8 @@ class SharedMemoryObject : public DesignPattern::Creation<SharedMemoryObject, Sh
                        const AccessMode accessMode,
                        const OpenMode openMode,
                        const cxx::optional<const void*>& baseAddressHint = cxx::nullopt,
-                       const mode_t permissions = S_IRUSR | S_IWUSR | S_IRGRP | S_IWGRP) noexcept;
+                       const cxx::perms permissions = cxx::perms::owner_read | cxx::perms::owner_write
+                                                      | cxx::perms::group_read | cxx::perms::group_write) noexcept;
 
     bool isInitialized() const noexcept;
 

iceoryx_hoofs/include/iceoryx_hoofs/internal/posix_wrapper/shared_memory_object/shared_memory.hpp

@@ -1,5 +1,5 @@
 // Copyright (c) 2019 by Robert Bosch GmbH. All rights reserved.
-// Copyright (c) 2021 by Apex.AI Inc. All rights reserved.
+// Copyright (c) 2021 - 2022 by Apex.AI Inc. All rights reserved.
 //
 // Licensed under the Apache License, Version 2.0 (the "License");
 // you may not use this file except in compliance with the License.
@@ -17,10 +17,11 @@
 #ifndef IOX_HOOFS_POSIX_WRAPPER_SHARED_MEMORY_OBJECT_SHARED_MEMORY_HPP
 #define IOX_HOOFS_POSIX_WRAPPER_SHARED_MEMORY_OBJECT_SHARED_MEMORY_HPP
 
+#include "iceoryx_hoofs/cxx/expected.hpp"
+#include "iceoryx_hoofs/cxx/filesystem.hpp"
+#include "iceoryx_hoofs/cxx/helplets.hpp"
 #include "iceoryx_hoofs/cxx/optional.hpp"
 #include "iceoryx_hoofs/cxx/string.hpp"
-#include "iceoryx_hoofs/design_pattern/creation.hpp"
-#include "iceoryx_hoofs/platform/mman.hpp"
 
 #include <cstdint>
 
@@ -54,7 +55,7 @@ static constexpr const char* OPEN_MODE_STRING[] = {
 enum class SharedMemoryError
 {
     EMPTY_NAME,
-    NAME_WITHOUT_LEADING_SLASH,
+    INVALID_FILE_NAME,
     INSUFFICIENT_PERMISSIONS,
     DOES_EXIST,
     PROCESS_LIMIT_OF_OPEN_FILES_REACHED,
@@ -74,7 +75,7 @@ enum class SharedMemoryError
 ///        shm_open, shm_unlink etc.
 ///        It must be used in combination with MemoryMap (or manual mmap calls)
 //         to gain access to the created/opened shared memory
-class SharedMemory : public DesignPattern::Creation<SharedMemory, SharedMemoryError>
+class SharedMemory
 {
   public:
     static constexpr uint64_t NAME_SIZE = platform::IOX_MAX_SHM_NAME_LENGTH;
@@ -103,35 +104,50 @@ class SharedMemory : public DesignPattern::Creation<SharedMemory, SharedMemoryEr
     ///         SharedMemoryError when the underlying shm_unlink call failed.
     static cxx::expected<bool, SharedMemoryError> unlinkIfExist(const Name_t& name) noexcept;
 
-    friend class DesignPattern::Creation<SharedMemory, SharedMemoryError>;
+    friend class SharedMemoryBuilder;
 
   private:
-    /// @brief constructs or opens existing shared memory
-    /// @param[in] name the name of the shared memory, must start with a leading /
-    /// @param[in] accessMode defines if the shared memory is mapped read only or with read write rights
-    /// @param[in] openMode states how the shared memory is created/opened
-    /// @param[in] permissions the permissions the shared memory should have
-    /// @param[in] size the size in bytes of the shared memory
-    SharedMemory(const Name_t& name,
-                 const AccessMode accessMode,
-                 const OpenMode openMode,
-                 const mode_t permissions,
-                 const uint64_t size) noexcept;
-
-    bool
-    open(const AccessMode accessMode, const OpenMode openMode, const mode_t permissions, const uint64_t size) noexcept;
+    SharedMemory(const Name_t& name, const int handle, const bool hasOwnership) noexcept;
+
     bool unlink() noexcept;
     bool close() noexcept;
     void destroy() noexcept;
     void reset() noexcept;
-    static int getOflagsFor(const AccessMode accessMode, const OpenMode openMode) noexcept;
 
     static SharedMemoryError errnoToEnum(const int32_t errnum) noexcept;
 
     Name_t m_name;
     int m_handle{INVALID_HANDLE};
     bool m_hasOwnership{false};
 };
+
+class SharedMemoryBuilder
+{
+    /// @brief A valid file name for the shared memory with the restriction that
+    ///        no leading dot is allowed since it is not compatible with every
+    ///        file system
+    IOX_BUILDER_PARAMETER(SharedMemory::Name_t, name, "")
+
+    /// @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 written to.
+    IOX_BUILDER_PARAMETER(AccessMode, accessMode, AccessMode::READ_ONLY)
+
+    /// @brief Defines how the shared memory is acquired
+    IOX_BUILDER_PARAMETER(OpenMode, openMode, OpenMode::OPEN_EXISTING)
+
+    /// @brief Defines the access permissions of the shared memory
+    IOX_BUILDER_PARAMETER(cxx::perms, filePermissions, cxx::perms::none)
+
+    /// @brief Defines the size of the shared memory
+    IOX_BUILDER_PARAMETER(uint64_t, size, 0U)
+
+  public:
+    /// @brief creates a valid SharedMemory object. If the construction failed the expected
+    ///        contains an enum value describing the error.
+    /// @return expected containing SharedMemory on success otherwise SharedMemoryError
+    cxx::expected<SharedMemory, SharedMemoryError> create() noexcept;
+};
+
 } // namespace posix
 } // namespace iox
 

iceoryx_hoofs/include/iceoryx_hoofs/posix_wrapper/named_pipe.hpp

@@ -40,7 +40,7 @@ class NamedPipe : public DesignPattern::Creation<NamedPipe, IpcChannelError>
 
     static constexpr uint64_t NULL_TERMINATOR_SIZE = 0U;
     static constexpr units::Duration CYCLE_TIME = units::Duration::fromMilliseconds(10);
-    static constexpr const char NAMED_PIPE_PREFIX[] = "/iox_np_";
+    static constexpr const char NAMED_PIPE_PREFIX[] = "iox_np_";
 
     using Message_t = cxx::string<MAX_MESSAGE_SIZE>;
     using MessageQueue_t = concurrent::LockFreeQueue<Message_t, MAX_NUMBER_OF_MESSAGES>;

iceoryx_hoofs/source/posix_wrapper/named_pipe.cpp

@@ -184,17 +184,13 @@ cxx::expected<bool, IpcChannelError> NamedPipe::isOutdated() noexcept
 
 cxx::expected<bool, IpcChannelError> NamedPipe::unlinkIfExists(const IpcChannelName_t& name) noexcept
 {
-    constexpr int ERROR_CODE = -1;
-    auto unlinkCall = posixCall(iox_shm_unlink)(convertName(NAMED_PIPE_PREFIX, name).c_str())
-                          .failureReturnValue(ERROR_CODE)
-                          .ignoreErrnos(ENOENT)
-                          .evaluate();
-    if (!unlinkCall.has_error())
+    auto result = SharedMemory::unlinkIfExist(convertName(NAMED_PIPE_PREFIX, name));
+    if (result.has_error())
     {
-        return cxx::success<bool>(unlinkCall->errnum != ENOENT);
+        return cxx::error<IpcChannelError>(IpcChannelError::INTERNAL_LOGIC_ERROR);
     }
 
-    return cxx::error<IpcChannelError>(IpcChannelError::INTERNAL_LOGIC_ERROR);
+    return cxx::success<bool>(*result);
 }
 
 cxx::expected<IpcChannelError> NamedPipe::trySend(const std::string& message) const noexcept

iceoryx_hoofs/source/posix_wrapper/shared_memory_object.cpp

@@ -52,12 +52,18 @@ SharedMemoryObject::SharedMemoryObject(const SharedMemory::Name_t& name,
                                        const AccessMode accessMode,
                                        const OpenMode openMode,
                                        const cxx::optional<const void*>& baseAddressHint,
-                                       const mode_t permissions) noexcept
+                                       const cxx::perms permissions) noexcept
     : m_memorySizeInBytes(cxx::align(memorySizeInBytes, Allocator::MEMORY_ALIGNMENT))
 {
     m_isInitialized = true;
 
-    SharedMemory::create(name, accessMode, openMode, permissions, m_memorySizeInBytes)
+    SharedMemoryBuilder()
+        .name(name)
+        .accessMode(accessMode)
+        .openMode(openMode)
+        .filePermissions(permissions)
+        .size(m_memorySizeInBytes)
+        .create()
         .and_then([this](auto& sharedMemory) { m_sharedMemory.emplace(std::move(sharedMemory)); })
         .or_else([this](auto&) {
             std::cerr << "Unable to create SharedMemoryObject since we could not acquire a SharedMemory resource"
@@ -99,7 +105,8 @@ SharedMemoryObject::SharedMemoryObject(const SharedMemory::Name_t& name,
         {
             std::cerr << "no hint set";
         }
-        std::cerr << ", permissions = " << std::bitset<sizeof(mode_t)>(permissions) << " ]" << std::endl;
+        std::cerr << ", permissions = " << std::bitset<sizeof(mode_t)>(static_cast<mode_t>(permissions)) << " ]"
+                  << std::endl;
         std::cerr.setf(flags);
         return;
     }
@@ -128,7 +135,7 @@ SharedMemoryObject::SharedMemoryObject(const SharedMemory::Name_t& name,
                 ACCESS_MODE_STRING[static_cast<uint64_t>(accessMode)],
                 OPEN_MODE_STRING[static_cast<uint64_t>(openMode)],
                 (baseAddressHint) ? *baseAddressHint : nullptr,
-                std::bitset<sizeof(mode_t)>(permissions).to_ulong());
+                std::bitset<sizeof(mode_t)>(static_cast<mode_t>(permissions)).to_ulong());
 
             memset(m_memoryMap->getBaseAddress(), 0, m_memorySizeInBytes);
         }