diff --git a/CHANGELOG.md b/CHANGELOG.md index 400e450f7e..4303183a80 100644 --- a/CHANGELOG.md +++ b/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:** diff --git a/iceoryx_hoofs/include/iceoryx_hoofs/cxx/helplets.hpp b/iceoryx_hoofs/include/iceoryx_hoofs/cxx/helplets.hpp index 34ef912cd0..2a35164cf4 100644 --- a/iceoryx_hoofs/include/iceoryx_hoofs/cxx/helplets.hpp +++ b/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 diff --git a/iceoryx_hoofs/include/iceoryx_hoofs/internal/posix_wrapper/shared_memory_object.hpp b/iceoryx_hoofs/include/iceoryx_hoofs/internal/posix_wrapper/shared_memory_object.hpp index 9ffb60ada3..996ea7dd4e 100644 --- a/iceoryx_hoofs/include/iceoryx_hoofs/internal/posix_wrapper/shared_memory_object.hpp +++ b/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& 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; diff --git a/iceoryx_hoofs/include/iceoryx_hoofs/internal/posix_wrapper/shared_memory_object/shared_memory.hpp b/iceoryx_hoofs/include/iceoryx_hoofs/internal/posix_wrapper/shared_memory_object/shared_memory.hpp index 6432bf0fde..6818922954 100644 --- a/iceoryx_hoofs/include/iceoryx_hoofs/internal/posix_wrapper/shared_memory_object/shared_memory.hpp +++ b/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 @@ -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 +class SharedMemory { public: static constexpr uint64_t NAME_SIZE = platform::IOX_MAX_SHM_NAME_LENGTH; @@ -103,28 +104,15 @@ class SharedMemory : public DesignPattern::Creation unlinkIfExist(const Name_t& name) noexcept; - friend class DesignPattern::Creation; + 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; @@ -132,6 +120,34 @@ class SharedMemory : public DesignPattern::Creation create() noexcept; +}; + } // namespace posix } // namespace iox diff --git a/iceoryx_hoofs/include/iceoryx_hoofs/posix_wrapper/named_pipe.hpp b/iceoryx_hoofs/include/iceoryx_hoofs/posix_wrapper/named_pipe.hpp index da2474e54c..c414a244f7 100644 --- a/iceoryx_hoofs/include/iceoryx_hoofs/posix_wrapper/named_pipe.hpp +++ b/iceoryx_hoofs/include/iceoryx_hoofs/posix_wrapper/named_pipe.hpp @@ -40,7 +40,7 @@ class NamedPipe : public DesignPattern::Creation 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; using MessageQueue_t = concurrent::LockFreeQueue; diff --git a/iceoryx_hoofs/source/posix_wrapper/named_pipe.cpp b/iceoryx_hoofs/source/posix_wrapper/named_pipe.cpp index 3a807e4247..1ab7bf20e9 100644 --- a/iceoryx_hoofs/source/posix_wrapper/named_pipe.cpp +++ b/iceoryx_hoofs/source/posix_wrapper/named_pipe.cpp @@ -184,17 +184,13 @@ cxx::expected NamedPipe::isOutdated() noexcept cxx::expected 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(unlinkCall->errnum != ENOENT); + return cxx::error(IpcChannelError::INTERNAL_LOGIC_ERROR); } - return cxx::error(IpcChannelError::INTERNAL_LOGIC_ERROR); + return cxx::success(*result); } cxx::expected NamedPipe::trySend(const std::string& message) const noexcept diff --git a/iceoryx_hoofs/source/posix_wrapper/shared_memory_object.cpp b/iceoryx_hoofs/source/posix_wrapper/shared_memory_object.cpp index 127a8be1c5..33d352d57c 100644 --- a/iceoryx_hoofs/source/posix_wrapper/shared_memory_object.cpp +++ b/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& 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(permissions) << " ]" << std::endl; + std::cerr << ", permissions = " << std::bitset(static_cast(permissions)) << " ]" + << std::endl; std::cerr.setf(flags); return; } @@ -128,7 +135,7 @@ SharedMemoryObject::SharedMemoryObject(const SharedMemory::Name_t& name, ACCESS_MODE_STRING[static_cast(accessMode)], OPEN_MODE_STRING[static_cast(openMode)], (baseAddressHint) ? *baseAddressHint : nullptr, - std::bitset(permissions).to_ulong()); + std::bitset(static_cast(permissions)).to_ulong()); memset(m_memoryMap->getBaseAddress(), 0, m_memorySizeInBytes); } diff --git a/iceoryx_hoofs/source/posix_wrapper/shared_memory_object/shared_memory.cpp b/iceoryx_hoofs/source/posix_wrapper/shared_memory_object/shared_memory.cpp index 4e312a1de3..3a59f75c82 100644 --- a/iceoryx_hoofs/source/posix_wrapper/shared_memory_object/shared_memory.cpp +++ b/iceoryx_hoofs/source/posix_wrapper/shared_memory_object/shared_memory.cpp @@ -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. @@ -33,73 +33,151 @@ namespace iox { namespace posix { -// NOLINTNEXTLINE(readability-function-size) todo(iox-#832): make a struct out of arguments -SharedMemory::SharedMemory(const Name_t& name, - const AccessMode accessMode, - const OpenMode openMode, - const mode_t permissions, - const uint64_t size) noexcept +static int getOflagsFor(const AccessMode accessMode, const OpenMode openMode) noexcept { - m_isInitialized = true; + int oflags = 0; + oflags |= (accessMode == AccessMode::READ_ONLY) ? O_RDONLY : O_RDWR; + oflags |= (openMode != OpenMode::OPEN_EXISTING) ? O_CREAT : 0; + oflags |= (openMode == OpenMode::EXCLUSIVE_CREATE) ? O_EXCL : 0; + return oflags; +} + +cxx::string addLeadingSlash(const SharedMemory::Name_t& name) noexcept +{ + cxx::string nameWithLeadingSlash = "/"; + nameWithLeadingSlash.append(cxx::TruncateToCapacity, name); + return nameWithLeadingSlash; +} + +cxx::expected SharedMemoryBuilder::create() noexcept +{ + auto printError = [this] { + std::cerr << "Unable to create shared memory with the following properties [ name = " << m_name + << ", access mode = " << ACCESS_MODE_STRING[static_cast(m_accessMode)] + << ", open mode = " << OPEN_MODE_STRING[static_cast(m_openMode)] + << ", mode = " << std::bitset(static_cast(m_filePermissions)) + << ", sizeInBytes = " << m_size << " ]" << std::endl; + }; + + // on qnx the current working directory will be added to the /dev/shmem path if the leading slash is missing - if (name.empty()) + if (m_name.empty()) { std::cerr << "No shared memory name specified!" << std::endl; - m_isInitialized = false; - m_errorValue = SharedMemoryError::EMPTY_NAME; + return cxx::error(SharedMemoryError::EMPTY_NAME); } - // empty case handled above, so it's fine to get first element - // NOLINTNEXTLINE(cppcoreguidelines-pro-bounds-pointer-arithmetic) - else if (name.c_str()[0] != '/') + + if (!cxx::isValidFileName(m_name)) { - std::cerr << "Shared memory name must start with a leading slash!" << std::endl; - m_isInitialized = false; - m_errorValue = SharedMemoryError::NAME_WITHOUT_LEADING_SLASH; + std::cerr << "Shared memory requires a valid file name (not path) as name and \"" << m_name + << "\" is not a valid file name" << std::endl; + return cxx::error(SharedMemoryError::INVALID_FILE_NAME); } - if (m_isInitialized) + auto nameWithLeadingSlash = addLeadingSlash(m_name); + + // the mask will be applied to the permissions, therefore we need to set it to 0 + int sharedMemoryFileHandle = SharedMemory::INVALID_HANDLE; + mode_t umaskSaved = umask(0U); { - m_name = name; - m_isInitialized = open(accessMode, openMode, permissions, size); + cxx::GenericRAII umaskGuard([&] { umask(umaskSaved); }); + + if (m_openMode == OpenMode::PURGE_AND_CREATE) + { + IOX_DISCARD_RESULT(posixCall(iox_shm_unlink)(nameWithLeadingSlash.c_str()) + .failureReturnValue(SharedMemory::INVALID_HANDLE) + .ignoreErrnos(ENOENT) + .evaluate()); + } + + auto result = + posixCall(iox_shm_open)( + nameWithLeadingSlash.c_str(), + getOflagsFor(m_accessMode, + (m_openMode == OpenMode::OPEN_OR_CREATE) ? OpenMode::EXCLUSIVE_CREATE : m_openMode), + static_cast(m_filePermissions)) + .failureReturnValue(SharedMemory::INVALID_HANDLE) + .suppressErrorMessagesForErrnos((m_openMode == OpenMode::OPEN_OR_CREATE) ? EEXIST : 0) + .evaluate(); + if (result.has_error()) + { + // if it was not possible to create the shm exclusively someone else has the + // ownership and we just try to open it + if (m_openMode == OpenMode::OPEN_OR_CREATE && result.get_error().errnum == EEXIST) + { + result = posixCall(iox_shm_open)(nameWithLeadingSlash.c_str(), + getOflagsFor(m_accessMode, OpenMode::OPEN_EXISTING), + static_cast(m_filePermissions)) + .failureReturnValue(SharedMemory::INVALID_HANDLE) + .evaluate(); + if (!result.has_error()) + { + constexpr bool HAS_NO_OWNERSHIP = false; + sharedMemoryFileHandle = result->value; + return cxx::success(SharedMemory(m_name, sharedMemoryFileHandle, HAS_NO_OWNERSHIP)); + } + } + + printError(); + return cxx::error(SharedMemory::errnoToEnum(result.get_error().errnum)); + } + sharedMemoryFileHandle = result->value; } - if (!m_isInitialized) + const bool hasOwnership = (m_openMode == OpenMode::EXCLUSIVE_CREATE || m_openMode == OpenMode::PURGE_AND_CREATE + || m_openMode == OpenMode::OPEN_OR_CREATE); + if (hasOwnership) { - std::cerr << "Unable to create shared memory with the following properties [ name = " << name - << ", access mode = " << ACCESS_MODE_STRING[static_cast(accessMode)] - << ", open mode = " << OPEN_MODE_STRING[static_cast(openMode)] - << ", mode = " << std::bitset(permissions) << ", sizeInBytes = " << size << " ]" - << std::endl; - return; + auto result = posixCall(ftruncate)(sharedMemoryFileHandle, static_cast(m_size)) + .failureReturnValue(SharedMemory::INVALID_HANDLE) + .evaluate(); + if (result.has_error()) + { + printError(); + + posixCall(iox_close)(sharedMemoryFileHandle) + .failureReturnValue(SharedMemory::INVALID_HANDLE) + .evaluate() + .or_else([&](auto& r) { + std::cerr << "Unable to close filedescriptor (close failed) : " << r.getHumanReadableErrnum() + << " for SharedMemory \"" << m_name << "\"" << std::endl; + }); + + posixCall(iox_shm_unlink)(nameWithLeadingSlash.c_str()) + .failureReturnValue(SharedMemory::INVALID_HANDLE) + .evaluate() + .or_else([&](auto&) { + std::cerr << "Unable to remove previously created SharedMemory \"" << m_name + << "\". This may be a SharedMemory leak." << std::endl; + }); + + return cxx::error(SharedMemory::errnoToEnum(result->errnum)); + } } + + return cxx::success(SharedMemory(m_name, sharedMemoryFileHandle, hasOwnership)); } -SharedMemory::~SharedMemory() noexcept +SharedMemory::SharedMemory(const Name_t& name, const int handle, const bool hasOwnership) noexcept + : m_name{name} + , m_handle{handle} + , m_hasOwnership{hasOwnership} { - destroy(); } -int SharedMemory::getOflagsFor(const AccessMode accessMode, const OpenMode openMode) noexcept +SharedMemory::~SharedMemory() noexcept { - int oflags = 0; - oflags |= (accessMode == AccessMode::READ_ONLY) ? O_RDONLY : O_RDWR; - oflags |= (openMode != OpenMode::OPEN_EXISTING) ? O_CREAT : 0; - oflags |= (openMode == OpenMode::EXCLUSIVE_CREATE) ? O_EXCL : 0; - return oflags; + destroy(); } void SharedMemory::destroy() noexcept { - if (m_isInitialized) - { - close(); - unlink(); - } + close(); + unlink(); } void SharedMemory::reset() noexcept { - m_isInitialized = false; m_hasOwnership = false; m_name = Name_t(); m_handle = INVALID_HANDLE; @@ -116,8 +194,6 @@ SharedMemory& SharedMemory::operator=(SharedMemory&& rhs) noexcept { destroy(); - CreationPattern_t::operator=(std::move(rhs)); - m_name = rhs.m_name; m_hasOwnership = std::move(rhs.m_hasOwnership); m_handle = std::move(rhs.m_handle); @@ -137,80 +213,14 @@ bool SharedMemory::hasOwnership() const noexcept return m_hasOwnership; } -// NOLINTNEXTLINE(readability-function-size) todo(iox-#832): make a struct out of arguments -bool SharedMemory::open(const AccessMode accessMode, - const OpenMode openMode, - const mode_t permissions, - const uint64_t size) noexcept -{ - cxx::Expects(size <= static_cast(std::numeric_limits::max())); - - m_hasOwnership = (openMode == OpenMode::EXCLUSIVE_CREATE || openMode == OpenMode::PURGE_AND_CREATE - || openMode == OpenMode::OPEN_OR_CREATE); - // the mask will be applied to the permissions, therefore we need to set it to 0 - mode_t umaskSaved = umask(0U); - { - cxx::GenericRAII umaskGuard([&] { umask(umaskSaved); }); - - if (openMode == OpenMode::PURGE_AND_CREATE) - { - IOX_DISCARD_RESULT(posixCall(iox_shm_unlink)(m_name.c_str()) - .failureReturnValue(INVALID_HANDLE) - .ignoreErrnos(ENOENT) - .evaluate()); - } - - auto result = posixCall(iox_shm_open)( - m_name.c_str(), - getOflagsFor(accessMode, - (openMode == OpenMode::OPEN_OR_CREATE) ? OpenMode::EXCLUSIVE_CREATE : openMode), - permissions) - .failureReturnValue(INVALID_HANDLE) - .suppressErrorMessagesForErrnos((openMode == OpenMode::OPEN_OR_CREATE) ? EEXIST : 0) - .evaluate(); - if (result.has_error()) - { - // if it was not possible to create the shm exclusively someone else has the - // ownership and we just try to open it - if (openMode == OpenMode::OPEN_OR_CREATE && result.get_error().errnum == EEXIST) - { - m_hasOwnership = false; - result = posixCall(iox_shm_open)( - m_name.c_str(), getOflagsFor(accessMode, OpenMode::OPEN_EXISTING), permissions) - .failureReturnValue(INVALID_HANDLE) - .evaluate(); - if (!result.has_error()) - { - m_handle = result->value; - return true; - } - } - - m_errorValue = errnoToEnum(result.get_error().errnum); - return false; - } - m_handle = result->value; - } - - if (m_hasOwnership) - { - if (posixCall(ftruncate)(m_handle, static_cast(size)) - .failureReturnValue(INVALID_HANDLE) - .evaluate() - .or_else([this](auto& r) { m_errorValue = errnoToEnum(r.errnum); }) - .has_error()) - { - return false; - } - } - - return true; -} - cxx::expected SharedMemory::unlinkIfExist(const Name_t& name) noexcept { - auto result = - posixCall(iox_shm_unlink)(name.c_str()).failureReturnValue(INVALID_HANDLE).ignoreErrnos(ENOENT).evaluate(); + auto nameWithLeadingSlash = addLeadingSlash(name); + + auto result = posixCall(iox_shm_unlink)(nameWithLeadingSlash.c_str()) + .failureReturnValue(INVALID_HANDLE) + .ignoreErrnos(ENOENT) + .evaluate(); if (!result.has_error()) { @@ -222,7 +232,7 @@ cxx::expected SharedMemory::unlinkIfExist(const Name_t& bool SharedMemory::unlink() noexcept { - if (m_isInitialized && m_hasOwnership) + if (m_hasOwnership) { auto unlinkResult = unlinkIfExist(m_name); if (unlinkResult.has_error() || !unlinkResult.value()) @@ -230,6 +240,7 @@ bool SharedMemory::unlink() noexcept std::cerr << "Unable to unlink SharedMemory (shm_unlink failed)." << std::endl; return false; } + m_hasOwnership = false; } reset(); @@ -238,7 +249,7 @@ bool SharedMemory::unlink() noexcept bool SharedMemory::close() noexcept { - if (m_isInitialized) + if (m_handle != INVALID_HANDLE) { auto call = posixCall(iox_close)(m_handle).failureReturnValue(INVALID_HANDLE).evaluate().or_else([](auto& r) { std::cerr << "Unable to close SharedMemory filedescriptor (close failed) : " << r.getHumanReadableErrnum() diff --git a/iceoryx_hoofs/test/moduletests/test_cxx_filesystem.cpp b/iceoryx_hoofs/test/moduletests/test_cxx_filesystem.cpp index 6d68367efb..81dcb31ddd 100644 --- a/iceoryx_hoofs/test/moduletests/test_cxx_filesystem.cpp +++ b/iceoryx_hoofs/test/moduletests/test_cxx_filesystem.cpp @@ -174,6 +174,4 @@ TEST(filesystem_test, streamOperatorPrintsCorrectlyWhenSetToUnknown) ASSERT_THAT(loggerMock.m_logs.size(), Eq(1U)); EXPECT_THAT(loggerMock.m_logs[0].message, Eq("unknown permissions")); } - - } // namespace diff --git a/iceoryx_hoofs/test/moduletests/test_shared_memory.cpp b/iceoryx_hoofs/test/moduletests/test_shared_memory.cpp index 5d424f8a8b..f64e064b2e 100644 --- a/iceoryx_hoofs/test/moduletests/test_shared_memory.cpp +++ b/iceoryx_hoofs/test/moduletests/test_shared_memory.cpp @@ -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. @@ -18,6 +18,7 @@ #include "test.hpp" #include "iceoryx_hoofs/internal/posix_wrapper/shared_memory_object/shared_memory.hpp" +#include "iceoryx_hoofs/platform/mman.hpp" #include "iceoryx_hoofs/platform/stat.hpp" #include "iceoryx_hoofs/posix_wrapper/posix_call.hpp" @@ -48,22 +49,24 @@ class SharedMemory_Test : public Test } } - static constexpr const char SUT_SHM_NAME[] = "/ignatz"; + static constexpr const char SUT_SHM_NAME[] = "ignatz"; iox::cxx::expected createSut(const iox::posix::SharedMemory::Name_t& name, const iox::posix::OpenMode openMode) { - return iox::posix::SharedMemory::create(name, - iox::posix::AccessMode::READ_WRITE, - openMode, - S_IRUSR | S_IWUSR | S_IRGRP | S_IWGRP | S_IROTH | S_IWOTH, - 128); + return iox::posix::SharedMemoryBuilder() + .name(name) + .accessMode(iox::posix::AccessMode::READ_WRITE) + .openMode(openMode) + .filePermissions(cxx::perms::owner_all) + .size(128) + .create(); } bool createRawSharedMemory(const iox::posix::SharedMemory::Name_t& name) { return !iox::posix::posixCall(iox_shm_open)( - name.c_str(), O_RDWR | O_CREAT, S_IRUSR | S_IWUSR | S_IRGRP | S_IWGRP) + (std::string("/") + name.c_str()).c_str(), O_RDWR | O_CREAT, S_IRUSR | S_IWUSR | S_IRGRP | S_IWGRP) .failureReturnValue(SharedMemory::INVALID_HANDLE) .evaluate() .has_error(); @@ -94,7 +97,7 @@ TEST_F(SharedMemory_Test, CTorWithInvalidMessageQueueNames) { ::testing::Test::RecordProperty("TEST_ID", "76ed82b1-eef7-4a5a-8794-b333c679e726"); EXPECT_THAT(createSut("", iox::posix::OpenMode::PURGE_AND_CREATE).has_error(), Eq(true)); - EXPECT_THAT(createSut("ignatz", iox::posix::OpenMode::PURGE_AND_CREATE).has_error(), Eq(true)); + EXPECT_THAT(createSut("/ignatz", iox::posix::OpenMode::PURGE_AND_CREATE).has_error(), Eq(true)); } TEST_F(SharedMemory_Test, CTorWithInvalidArguments) @@ -115,7 +118,6 @@ TEST_F(SharedMemory_Test, MoveCTorWithValidValues) { iox::posix::SharedMemory sut2(std::move(*sut)); EXPECT_THAT(handle, Eq(sut2.getHandle())); - EXPECT_THAT(sut->isInitialized(), Eq(false)); } } @@ -138,7 +140,7 @@ TEST_F(SharedMemory_Test, UnlinkNonExistingShmFails) TEST_F(SharedMemory_Test, UnlinkExistingShmWorks) { ::testing::Test::RecordProperty("TEST_ID", "11f0b2f2-b891-41e4-bb82-648a9541582f"); - constexpr const char SHM_NAME[] = "/its_a_mee_monukulius"; + constexpr const char SHM_NAME[] = "its_a_mee_monukulius"; ASSERT_TRUE(createRawSharedMemory(SHM_NAME)); auto result = iox::posix::SharedMemory::unlinkIfExist(SHM_NAME); ASSERT_FALSE(result.has_error()); diff --git a/iceoryx_hoofs/test/moduletests/test_shared_memory_object.cpp b/iceoryx_hoofs/test/moduletests/test_shared_memory_object.cpp index fd38159810..55c342aa34 100644 --- a/iceoryx_hoofs/test/moduletests/test_shared_memory_object.cpp +++ b/iceoryx_hoofs/test/moduletests/test_shared_memory_object.cpp @@ -54,7 +54,7 @@ class SharedMemoryObject_Test : public Test TEST_F(SharedMemoryObject_Test, CTorWithValidArguments) { ::testing::Test::RecordProperty("TEST_ID", "bbda60d2-d741-407e-9a9f-f0ca74d985a8"); - auto sut = iox::posix::SharedMemoryObject::create("/validShmMem", + auto sut = iox::posix::SharedMemoryObject::create("validShmMem", 100, iox::posix::AccessMode::READ_WRITE, iox::posix::OpenMode::PURGE_AND_CREATE, @@ -65,7 +65,7 @@ TEST_F(SharedMemoryObject_Test, CTorWithValidArguments) TEST_F(SharedMemoryObject_Test, CTorOpenNonExistingSharedMemoryObject) { ::testing::Test::RecordProperty("TEST_ID", "d80278c3-1dd8-409d-9162-f7f900892526"); - auto sut = iox::posix::SharedMemoryObject::create("/pummeluff", + auto sut = iox::posix::SharedMemoryObject::create("pummeluff", 100, iox::posix::AccessMode::READ_WRITE, iox::posix::OpenMode::OPEN_EXISTING, @@ -76,7 +76,7 @@ TEST_F(SharedMemoryObject_Test, CTorOpenNonExistingSharedMemoryObject) TEST_F(SharedMemoryObject_Test, AllocateMemoryInSharedMemoryAndReadIt) { ::testing::Test::RecordProperty("TEST_ID", "6169ac70-a08e-4a19-80e4-57f0d5f89233"); - auto sut = iox::posix::SharedMemoryObject::create("/shmAllocate", + auto sut = iox::posix::SharedMemoryObject::create("shmAllocate", 16, iox::posix::AccessMode::READ_WRITE, iox::posix::OpenMode::PURGE_AND_CREATE, @@ -90,7 +90,7 @@ TEST_F(SharedMemoryObject_Test, AllocateMemoryInSharedMemoryAndReadIt) TEST_F(SharedMemoryObject_Test, AllocateWholeSharedMemoryWithOneChunk) { ::testing::Test::RecordProperty("TEST_ID", "2def907e-683d-4aaa-a969-47b5468d5383"); - auto sut = iox::posix::SharedMemoryObject::create("/shmAllocate", + auto sut = iox::posix::SharedMemoryObject::create("shmAllocate", 8, iox::posix::AccessMode::READ_WRITE, iox::posix::OpenMode::PURGE_AND_CREATE, @@ -103,7 +103,7 @@ TEST_F(SharedMemoryObject_Test, AllocateWholeSharedMemoryWithOneChunk) TEST_F(SharedMemoryObject_Test, AllocateWholeSharedMemoryWithMultipleChunks) { ::testing::Test::RecordProperty("TEST_ID", "dd70c0aa-fef5-49ed-875c-4bb768894ae5"); - auto sut = iox::posix::SharedMemoryObject::create("/shmAllocate", + auto sut = iox::posix::SharedMemoryObject::create("shmAllocate", 8, iox::posix::AccessMode::READ_WRITE, iox::posix::OpenMode::PURGE_AND_CREATE, @@ -120,7 +120,7 @@ TEST_F(SharedMemoryObject_Test, AllocateTooMuchMemoryInSharedMemoryWithOneChunk) { ::testing::Test::RecordProperty("TEST_ID", "4b054aac-1d49-4260-afc0-908b184e0b12"); uint64_t memorySize{8u}; - auto sut = iox::posix::SharedMemoryObject::create("/shmAllocate", + auto sut = iox::posix::SharedMemoryObject::create("shmAllocate", memorySize, iox::posix::AccessMode::READ_WRITE, iox::posix::OpenMode::PURGE_AND_CREATE, @@ -133,7 +133,7 @@ TEST_F(SharedMemoryObject_Test, AllocateTooMuchSharedMemoryWithMultipleChunks) { ::testing::Test::RecordProperty("TEST_ID", "5bb3c7fc-0f15-4487-8479-b27d1d4a17d3"); uint64_t memorySize{8u}; - auto sut = iox::posix::SharedMemoryObject::create("/shmAllocate", + auto sut = iox::posix::SharedMemoryObject::create("shmAllocate", memorySize, iox::posix::AccessMode::READ_WRITE, iox::posix::OpenMode::PURGE_AND_CREATE, @@ -151,7 +151,7 @@ TEST_F(SharedMemoryObject_Test, AllocateTooMuchSharedMemoryWithMultipleChunks) TEST_F(SharedMemoryObject_Test, AllocateAfterFinalizeAllocation) { ::testing::Test::RecordProperty("TEST_ID", "e4711eaa-e811-41d4-927a-63384cdcb984"); - auto sut = iox::posix::SharedMemoryObject::create("/shmAllocate", + auto sut = iox::posix::SharedMemoryObject::create("shmAllocate", 8, iox::posix::AccessMode::READ_WRITE, iox::posix::OpenMode::PURGE_AND_CREATE, @@ -165,7 +165,7 @@ TEST_F(SharedMemoryObject_Test, OpeningSharedMemoryAndReadMultipleContents) { ::testing::Test::RecordProperty("TEST_ID", "14f77425-34aa-43d0-82dd-e05efd93464b"); uint64_t memorySize = 128; - auto shmMemory = iox::posix::SharedMemoryObject::create("/shmSut", + auto shmMemory = iox::posix::SharedMemoryObject::create("shmSut", memorySize, iox::posix::AccessMode::READ_WRITE, iox::posix::OpenMode::PURGE_AND_CREATE, @@ -175,7 +175,7 @@ TEST_F(SharedMemoryObject_Test, OpeningSharedMemoryAndReadMultipleContents) int* test2 = static_cast(shmMemory->allocate(sizeof(int), 1)); *test2 = 8912; - auto sut = iox::posix::SharedMemoryObject::create("/shmSut", + auto sut = iox::posix::SharedMemoryObject::create("shmSut", memorySize, iox::posix::AccessMode::READ_WRITE, iox::posix::OpenMode::OPEN_EXISTING, diff --git a/iceoryx_posh/include/iceoryx_posh/iceoryx_posh_types.hpp b/iceoryx_posh/include/iceoryx_posh/iceoryx_posh_types.hpp index 488920a00b..1e560fd118 100644 --- a/iceoryx_posh/include/iceoryx_posh/iceoryx_posh_types.hpp +++ b/iceoryx_posh/include/iceoryx_posh/iceoryx_posh_types.hpp @@ -209,7 +209,7 @@ constexpr const char ROUDI_LOCK_NAME[] = "iox-unique-roudi"; constexpr const char IPC_CHANNEL_ROUDI_NAME[] = "roudi"; /// shared memmory segment for the iceoryx managment data -constexpr const char SHM_NAME[] = "/iceoryx_mgmt"; +constexpr const char SHM_NAME[] = "iceoryx_mgmt"; // this is used by the UniquePortId constexpr uint16_t DEFAULT_UNIQUE_ROUDI_ID{0U}; diff --git a/iceoryx_posh/include/iceoryx_posh/internal/mepoo/mepoo_segment.inl b/iceoryx_posh/include/iceoryx_posh/internal/mepoo/mepoo_segment.inl index 939faf1318..f7f3fd26c9 100644 --- a/iceoryx_posh/include/iceoryx_posh/internal/mepoo/mepoo_segment.inl +++ b/iceoryx_posh/include/iceoryx_posh/internal/mepoo/mepoo_segment.inl @@ -68,17 +68,14 @@ inline SharedMemoryObjectType MePooSegment(S_IRUSR | S_IWUSR | S_IRGRP | S_IWGRP)) + cxx::perms::owner_read | cxx::perms::owner_write | cxx::perms::group_read + | cxx::perms::group_write) .and_then([this](auto& sharedMemoryObject) { this->setSegmentId(iox::rp::BaseRelativePointer::registerPtr(sharedMemoryObject.getBaseAddress(), sharedMemoryObject.getSizeInBytes())); diff --git a/iceoryx_posh/include/iceoryx_posh/internal/mepoo/segment_manager.inl b/iceoryx_posh/include/iceoryx_posh/internal/mepoo/segment_manager.inl index 30d02041f4..98977fd09b 100644 --- a/iceoryx_posh/include/iceoryx_posh/internal/mepoo/segment_manager.inl +++ b/iceoryx_posh/include/iceoryx_posh/internal/mepoo/segment_manager.inl @@ -68,7 +68,7 @@ SegmentManager::getSegmentMappings(const posix::PosixUser& user) no // process if (!foundInWriterGroup) { - mappingContainer.emplace_back("/" + segment.getWriterGroup().getName(), + mappingContainer.emplace_back(segment.getWriterGroup().getName(), segment.getSharedMemoryObject().getBaseAddress(), segment.getSharedMemoryObject().getSizeInBytes(), true, @@ -94,7 +94,7 @@ SegmentManager::getSegmentMappings(const posix::PosixUser& user) no return mapping.m_startAddress == segment.getSharedMemoryObject().getBaseAddress(); }) == mappingContainer.end()) { - mappingContainer.emplace_back("/" + segment.getWriterGroup().getName(), + mappingContainer.emplace_back(segment.getWriterGroup().getName(), segment.getSharedMemoryObject().getBaseAddress(), segment.getSharedMemoryObject().getSizeInBytes(), false, diff --git a/iceoryx_posh/test/integrationtests/test_shm_sigbus_handler.cpp b/iceoryx_posh/test/integrationtests/test_shm_sigbus_handler.cpp index 040ffed5a7..68c8dd803a 100644 --- a/iceoryx_posh/test/integrationtests/test_shm_sigbus_handler.cpp +++ b/iceoryx_posh/test/integrationtests/test_shm_sigbus_handler.cpp @@ -32,7 +32,7 @@ using namespace ::testing; TEST(ShmCreatorDeathTest, AllocatingTooMuchMemoryLeadsToExitWithSIGBUS) { ::testing::Test::RecordProperty("TEST_ID", "d6c8949d-42c9-4b2c-a150-4306cb2a57f6"); - const iox::ShmName_t TEST_SHM_NAME{"/test_name"}; + const iox::ShmName_t TEST_SHM_NAME{"test_name"}; // the death test makes only sense on platforms which are zeroing the whole shared memory // if the memory is only reserved a death will never occur if (iox::platform::IOX_SHM_WRITE_ZEROS_ON_CREATION) diff --git a/iceoryx_posh/test/moduletests/test_mepoo_segment.cpp b/iceoryx_posh/test/moduletests/test_mepoo_segment.cpp index ff2339cd18..66062f9dea 100644 --- a/iceoryx_posh/test/moduletests/test_mepoo_segment.cpp +++ b/iceoryx_posh/test/moduletests/test_mepoo_segment.cpp @@ -47,13 +47,13 @@ class MePooSegment_test : public Test const iox::posix::AccessMode, const iox::posix::OpenMode, const void*, - const mode_t)>; + const iox::cxx::perms)>; SharedMemoryObject_MOCK(const SharedMemory::Name_t& name, const uint64_t memorySizeInBytes, const AccessMode accessMode, const OpenMode openMode, const void* baseAddressHint, - const mode_t permissions) + const iox::cxx::perms permissions) : m_memorySizeInBytes(memorySizeInBytes) , m_baseAddressHint(const_cast(baseAddressHint)) { @@ -142,8 +142,8 @@ TEST_F(MePooSegment_test, ADD_TEST_WITH_ADDITIONAL_USER(SharedMemoryCreationPara const iox::posix::AccessMode f_accessMode, const iox::posix::OpenMode openMode, const void*, - const mode_t) { - EXPECT_THAT(std::string(f_name), Eq(std::string("/iox_roudi_test2"))); + const iox::cxx::perms) { + EXPECT_THAT(std::string(f_name), Eq(std::string("iox_roudi_test2"))); EXPECT_THAT(f_accessMode, Eq(iox::posix::AccessMode::READ_WRITE)); EXPECT_THAT(openMode, Eq(iox::posix::OpenMode::PURGE_AND_CREATE)); }; @@ -162,7 +162,7 @@ TEST_F(MePooSegment_test, ADD_TEST_WITH_ADDITIONAL_USER(GetSharedMemoryObject)) const iox::posix::AccessMode, const iox::posix::OpenMode, const void*, - const mode_t) { + const iox::cxx::perms) { memorySizeInBytes = f_memorySizeInBytes; }; MePooSegment sut2{ diff --git a/iceoryx_posh/test/moduletests/test_roudi_posix_shm_memory_provider.cpp b/iceoryx_posh/test/moduletests/test_roudi_posix_shm_memory_provider.cpp index ebab3335b2..3811841fda 100644 --- a/iceoryx_posh/test/moduletests/test_roudi_posix_shm_memory_provider.cpp +++ b/iceoryx_posh/test/moduletests/test_roudi_posix_shm_memory_provider.cpp @@ -30,7 +30,7 @@ using namespace ::testing; using namespace iox::roudi; using iox::ShmName_t; -static const ShmName_t TEST_SHM_NAME = ShmName_t("/FuManchu"); +static const ShmName_t TEST_SHM_NAME = ShmName_t("FuManchu"); class PosixShmMemoryProvider_Test : public Test {