From 26394e66a6e6388de05e3d5505b1f98d68851305 Mon Sep 17 00:00:00 2001 From: Mathias Kraus Date: Tue, 13 Feb 2024 03:15:47 +0100 Subject: [PATCH] iox-#2185 Consolidate prefix creation --- .../iceoryx_posh/iceoryx_posh_types.hpp | 6 +++ .../iceoryx_posh/iceoryx_posh_types.inl | 10 +++++ .../internal/mepoo/mepoo_segment.inl | 10 ++--- .../memory/iceoryx_roudi_memory_manager.cpp | 41 ++++++++---------- .../memory/posix_shm_memory_provider.cpp | 7 +-- iceoryx_posh/source/roudi/roudi.cpp | 25 +++++++---- .../source/runtime/ipc_interface_base.cpp | 14 +++--- .../source/runtime/shared_memory_user.cpp | 17 ++------ .../test/moduletests/test_posh_types.cpp | 43 +++++++++++++++++++ .../test_roudi_posix_shm_memory_provider.cpp | 3 +- 10 files changed, 112 insertions(+), 64 deletions(-) create mode 100644 iceoryx_posh/test/moduletests/test_posh_types.cpp diff --git a/iceoryx_posh/include/iceoryx_posh/iceoryx_posh_types.hpp b/iceoryx_posh/include/iceoryx_posh/iceoryx_posh_types.hpp index 87aec0f96d..826a0999b1 100644 --- a/iceoryx_posh/include/iceoryx_posh/iceoryx_posh_types.hpp +++ b/iceoryx_posh/include/iceoryx_posh/iceoryx_posh_types.hpp @@ -20,6 +20,7 @@ #include "iceoryx_platform/platform_settings.hpp" #include "iceoryx_posh/iceoryx_posh_deployment.hpp" +#include "iox/detail/convert.hpp" #include "iox/duration.hpp" #include "iox/function.hpp" #include "iox/log/logstream.hpp" @@ -198,6 +199,11 @@ struct DefaultChunkQueueConfig constexpr const char ICEORYX_RESOURCE_PREFIX[] = "iox1"; +using ResourcePrefix_t = string<13>; // 'iox1_' + MAX_UINT16_SIZE + '_' + optional 'x_' +/// @brief Returns the prefix string used for resources +/// @param[in] uniqueRouDiID to use for the prefix string +inline ResourcePrefix_t iceoryxResourcePrefix(uint16_t uniqueRouDiID, iox::string<1> customizer = ""); + // alias for string using RuntimeName_t = string; using NodeName_t = string; diff --git a/iceoryx_posh/include/iceoryx_posh/iceoryx_posh_types.inl b/iceoryx_posh/include/iceoryx_posh/iceoryx_posh_types.inl index ecc99eb9d2..ebc09864ff 100644 --- a/iceoryx_posh/include/iceoryx_posh/iceoryx_posh_types.inl +++ b/iceoryx_posh/include/iceoryx_posh/iceoryx_posh_types.inl @@ -49,6 +49,16 @@ log::LogStream& operator<<(log::LogStream& stream, ConnectionState value) noexce return stream; } +ResourcePrefix_t iceoryxResourcePrefix(uint16_t uniqueRouDiID, iox::string<1> customizer) +{ + static_assert(std::is_same_v>); + constexpr auto MAX_UINT16_WIDTH{5}; + iox::string uniqueRoudiIdString{TruncateToCapacity, + iox::convert::toString(uniqueRouDiID).c_str()}; + auto end = customizer.empty() ? "_" : concatenate("_", customizer, "_"); + return concatenate(ICEORYX_RESOURCE_PREFIX, "_", uniqueRoudiIdString, end); +} + namespace roudi { inline iox::log::LogStream& operator<<(iox::log::LogStream& logstream, const MonitoringMode& mode) noexcept 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 2a3fb899ed..bf1bce3c29 100644 --- a/iceoryx_posh/include/iceoryx_posh/internal/mepoo/mepoo_segment.inl +++ b/iceoryx_posh/include/iceoryx_posh/internal/mepoo/mepoo_segment.inl @@ -74,14 +74,10 @@ inline SharedMemoryObjectType MePooSegment uniqueRoudiIdString{TruncateToCapacity, - iox::convert::toString(roudi::DEFAULT_UNIQUE_ROUDI_ID).c_str()}; using ShmName_t = detail::PosixSharedMemory::Name_t; - ShmName_t shmName = concatenate(ICEORYX_RESOURCE_PREFIX, - "_", - uniqueRoudiIdString, - "_p_"); // add a '_p_' to prevent creating a payload segment with - // the same name as the management segment + ShmName_t shmName = iceoryxResourcePrefix(roudi::DEFAULT_UNIQUE_ROUDI_ID, + "p"); // use an additional 'p' to prevent creating a payload + // segment with the same name as the management segment if (shmName.size() + writerGroup.getName().size() > ShmName_t::capacity()) { IOX_LOG(FATAL, diff --git a/iceoryx_posh/source/roudi/memory/iceoryx_roudi_memory_manager.cpp b/iceoryx_posh/source/roudi/memory/iceoryx_roudi_memory_manager.cpp index 7de2f02bdf..31f4d7f966 100644 --- a/iceoryx_posh/source/roudi/memory/iceoryx_roudi_memory_manager.cpp +++ b/iceoryx_posh/source/roudi/memory/iceoryx_roudi_memory_manager.cpp @@ -23,29 +23,24 @@ namespace iox namespace roudi { IceOryxRouDiMemoryManager::IceOryxRouDiMemoryManager(const RouDiConfig_t& roudiConfig) noexcept - : m_fileLock(std::move( - FileLockBuilder() - .name([] { - iox::string<1> uniqueRoudiIdString{TruncateToCapacity, - iox::convert::toString(DEFAULT_UNIQUE_ROUDI_ID).c_str()}; - auto lockName = concatenate(ICEORYX_RESOURCE_PREFIX, "_", uniqueRoudiIdString, "_", ROUDI_LOCK_NAME); - return lockName; - }()) - .permission(iox::perms::owner_read | iox::perms::owner_write) - .create() - .or_else([](auto& error) { - if (error == FileLockError::LOCKED_BY_OTHER_PROCESS) - { - IOX_LOG(FATAL, "Could not acquire lock, is RouDi still running?"); - IOX_REPORT_FATAL(PoshError::ICEORYX_ROUDI_MEMORY_MANAGER__ROUDI_STILL_RUNNING); - } - else - { - IOX_LOG(FATAL, "Error occurred while acquiring file lock named " << ROUDI_LOCK_NAME); - IOX_REPORT_FATAL(PoshError::ICEORYX_ROUDI_MEMORY_MANAGER__COULD_NOT_ACQUIRE_FILE_LOCK); - } - }) - .value())) + : m_fileLock( + std::move(FileLockBuilder() + .name(concatenate(iceoryxResourcePrefix(DEFAULT_UNIQUE_ROUDI_ID), ROUDI_LOCK_NAME)) + .permission(iox::perms::owner_read | iox::perms::owner_write) + .create() + .or_else([](auto& error) { + if (error == FileLockError::LOCKED_BY_OTHER_PROCESS) + { + IOX_LOG(FATAL, "Could not acquire lock, is RouDi still running?"); + IOX_REPORT_FATAL(PoshError::ICEORYX_ROUDI_MEMORY_MANAGER__ROUDI_STILL_RUNNING); + } + else + { + IOX_LOG(FATAL, "Error occurred while acquiring file lock named " << ROUDI_LOCK_NAME); + IOX_REPORT_FATAL(PoshError::ICEORYX_ROUDI_MEMORY_MANAGER__COULD_NOT_ACQUIRE_FILE_LOCK); + } + }) + .value())) , m_defaultMemory(roudiConfig) { m_defaultMemory.m_managementShm.addMemoryBlock(&m_portPoolBlock).or_else([](auto) { diff --git a/iceoryx_posh/source/roudi/memory/posix_shm_memory_provider.cpp b/iceoryx_posh/source/roudi/memory/posix_shm_memory_provider.cpp index ddd0cb4e5c..b5173aebee 100644 --- a/iceoryx_posh/source/roudi/memory/posix_shm_memory_provider.cpp +++ b/iceoryx_posh/source/roudi/memory/posix_shm_memory_provider.cpp @@ -57,12 +57,7 @@ expected PosixShmMemoryProvider::createMemory(const } if (!PosixSharedMemoryObjectBuilder() - .name([this] { - iox::string<1> uniqueRoudiIdString{TruncateToCapacity, - iox::convert::toString(DEFAULT_UNIQUE_ROUDI_ID).c_str()}; - auto shmName = concatenate(ICEORYX_RESOURCE_PREFIX, "_", uniqueRoudiIdString, "_", m_shmName); - return shmName; - }()) + .name(concatenate(iceoryxResourcePrefix(DEFAULT_UNIQUE_ROUDI_ID), m_shmName)) .memorySizeInBytes(size) .accessMode(m_accessMode) .openMode(m_openMode) diff --git a/iceoryx_posh/source/roudi/roudi.cpp b/iceoryx_posh/source/roudi/roudi.cpp index 3d837fda48..4c4d3f782e 100644 --- a/iceoryx_posh/source/roudi/roudi.cpp +++ b/iceoryx_posh/source/roudi/roudi.cpp @@ -289,19 +289,28 @@ void RouDi::processMessage(const runtime::IpcMessage& message, const iox::runtime::IpcMessageType& cmd, const RuntimeName_t& runtimeName) noexcept { - switch (cmd) + if (runtimeName.empty()) { - case runtime::IpcMessageType::REG: + IOX_LOG(ERROR, "Got message with empty runtime name!"); + return; + } + + + for (const auto s : platform::IOX_PATH_SEPARATORS) { - if (runtimeName.empty()) - { - IOX_LOG(ERROR, "Got message with empty runtime name!"); - } - else if (runtimeName.find(platform::IOX_PATH_SEPARATORS).has_value()) + const char separator[2]{s}; + if (runtimeName.find(separator).has_value()) { IOX_LOG(ERROR, "Got message with a runtime name with invalid characters: \"" << runtimeName << "\"!"); + return; } - else if (message.getNumberOfElements() != 6) + } + + switch (cmd) + { + case runtime::IpcMessageType::REG: + { + if (message.getNumberOfElements() != 6) { IOX_LOG(ERROR, "Wrong number of parameters for \"IpcMessageType::REG\" from \"" << runtimeName << "\"received!"); diff --git a/iceoryx_posh/source/runtime/ipc_interface_base.cpp b/iceoryx_posh/source/runtime/ipc_interface_base.cpp index 59ff53e1d8..205c8377ff 100644 --- a/iceoryx_posh/source/runtime/ipc_interface_base.cpp +++ b/iceoryx_posh/source/runtime/ipc_interface_base.cpp @@ -79,9 +79,7 @@ std::string IpcMessageErrorTypeToString(const IpcMessageErrorType msg) noexcept iox::optional ipcChannelNameToInterfaceName(RuntimeName_t channelName) { - iox::string<1> uniqueRoudiIdString{TruncateToCapacity, - iox::convert::toString(roudi::DEFAULT_UNIQUE_ROUDI_ID).c_str()}; - RuntimeName_t interfaceName = concatenate(ICEORYX_RESOURCE_PREFIX, "_", uniqueRoudiIdString, "_"); + RuntimeName_t interfaceName = iceoryxResourcePrefix(roudi::DEFAULT_UNIQUE_ROUDI_ID); if (interfaceName.size() + channelName.size() > RuntimeName_t::capacity()) { return nullopt; @@ -99,10 +97,14 @@ IpcInterface::IpcInterface(const RuntimeName_t& runtimeName, { IOX_PANIC("Then runtime name must not be empty"); } - else if (runtimeName.find(iox::platform::IOX_PATH_SEPARATORS).has_value()) + for (const auto s : platform::IOX_PATH_SEPARATORS) { - IOX_LOG(FATAL, "The runtime name '" << runtimeName << "' contains path separators"); - IOX_PANIC("Invalid characters for runtime name"); + const char separator[2]{s}; + if (runtimeName.find(separator).has_value()) + { + IOX_LOG(FATAL, "The runtime name '" << runtimeName << "' contains path separators"); + IOX_PANIC("Invalid characters for runtime name"); + } } m_interfaceName = diff --git a/iceoryx_posh/source/runtime/shared_memory_user.cpp b/iceoryx_posh/source/runtime/shared_memory_user.cpp index f8201425b7..4eb2315a83 100644 --- a/iceoryx_posh/source/runtime/shared_memory_user.cpp +++ b/iceoryx_posh/source/runtime/shared_memory_user.cpp @@ -33,12 +33,7 @@ SharedMemoryUser::SharedMemoryUser(const size_t topicSize, const UntypedRelativePointer::offset_t segmentManagerAddressOffset) noexcept { PosixSharedMemoryObjectBuilder() - .name([] { - iox::string<1> uniqueRoudiIdString{TruncateToCapacity, - iox::convert::toString(roudi::DEFAULT_UNIQUE_ROUDI_ID).c_str()}; - auto shmName = concatenate(ICEORYX_RESOURCE_PREFIX, "_", uniqueRoudiIdString, "_", roudi::SHM_NAME); - return shmName; - }()) + .name(concatenate(iceoryxResourcePrefix(roudi::DEFAULT_UNIQUE_ROUDI_ID), roudi::SHM_NAME)) .memorySizeInBytes(topicSize) .accessMode(AccessMode::READ_WRITE) .openMode(OpenMode::OPEN_EXISTING) @@ -80,14 +75,10 @@ void SharedMemoryUser::openDataSegments(const uint64_t segmentId, auto accessMode = segment.m_isWritable ? AccessMode::READ_WRITE : AccessMode::READ_ONLY; PosixSharedMemoryObjectBuilder() .name([&segment] { - iox::string<1> uniqueRoudiIdString{TruncateToCapacity, - iox::convert::toString(roudi::DEFAULT_UNIQUE_ROUDI_ID).c_str()}; using ShmName_t = detail::PosixSharedMemory::Name_t; - ShmName_t shmName = concatenate(ICEORYX_RESOURCE_PREFIX, - "_", - uniqueRoudiIdString, - "_p_"); // add a '_p_' to prevent creating a payload segment with the - // same name as the management segment + ShmName_t shmName = iceoryxResourcePrefix(roudi::DEFAULT_UNIQUE_ROUDI_ID, + "p"); // use an additional 'p' to prevent creating a payload + // segment with the same name as the management segment if (shmName.size() + segment.m_sharedMemoryName.size() > ShmName_t::capacity()) { IOX_LOG(FATAL, diff --git a/iceoryx_posh/test/moduletests/test_posh_types.cpp b/iceoryx_posh/test/moduletests/test_posh_types.cpp new file mode 100644 index 0000000000..7039b019d3 --- /dev/null +++ b/iceoryx_posh/test/moduletests/test_posh_types.cpp @@ -0,0 +1,43 @@ +// Copyright (c) 2024 by Mathias Kraus . 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. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +// +// SPDX-License-Identifier: Apache-2.0 + +#include "iceoryx_posh/iceoryx_posh_types.hpp" + +#include "test.hpp" + +using namespace ::testing; +using namespace iox; + +TEST(PoshTypes_test, IceoryxResourcePrefixWithDefaultRouDiIdWorks) +{ + ::testing::Test::RecordProperty("TEST_ID", "35f1d638-8efa-41dd-859b-bcc23450844f"); + + EXPECT_THAT(iceoryxResourcePrefix(roudi::DEFAULT_UNIQUE_ROUDI_ID).c_str(), StrEq("iox1_0_")); +} + +TEST(PoshTypes_test, IceoryxResourcePrefixWithMaxRouDiIdWorks) +{ + ::testing::Test::RecordProperty("TEST_ID", "049e79d7-d0ca-4951-8d44-c80aebab7a88"); + + EXPECT_THAT(iceoryxResourcePrefix(std::numeric_limits::max()).c_str(), StrEq("iox1_65535_")); +} + +TEST(PoshTypes_test, IceoryxResourcePrefixWithMaxRouDiIdAndCustomizerWorks) +{ + ::testing::Test::RecordProperty("TEST_ID", "b63bbdca-ff19-41bc-9f8a-c657b0ee8009"); + + EXPECT_THAT(iceoryxResourcePrefix(std::numeric_limits::max(), "a").c_str(), StrEq("iox1_65535_a_")); +} 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 7133c96bbd..b1993abff9 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 @@ -27,6 +27,7 @@ namespace { using namespace ::testing; +using namespace iox; using namespace iox::roudi; using iox::ShmName_t; @@ -48,7 +49,7 @@ class PosixShmMemoryProvider_Test : public Test bool shmExists() { return !iox::PosixSharedMemoryObjectBuilder() - .name(TEST_SHM_NAME) + .name(concatenate(iceoryxResourcePrefix(DEFAULT_UNIQUE_ROUDI_ID), TEST_SHM_NAME)) .memorySizeInBytes(8) .accessMode(iox::AccessMode::READ_ONLY) .openMode(iox::OpenMode::OPEN_EXISTING)