From b0c6ef4280a166f4982c344abe7cb96be3ebbc01 Mon Sep 17 00:00:00 2001 From: Mathias Kraus Date: Fri, 1 Sep 2023 20:05:47 +0200 Subject: [PATCH] iox-#1036 Make 'MessageQueue' and 'UnixDomainSocket' non-nullable --- .../posix_wrapper/message_queue.hpp | 6 +----- .../posix_wrapper/unix_domain_socket.hpp | 6 +----- .../moduletests/test_unix_domain_sockets.cpp | 17 ++++++----------- .../internal/runtime/ipc_interface_base.hpp | 3 ++- .../source/runtime/ipc_interface_base.cpp | 18 +++++++++--------- .../test_mq_interface_startup_race.cpp | 17 +++++++---------- 6 files changed, 26 insertions(+), 41 deletions(-) diff --git a/iceoryx_dust/include/iceoryx_dust/posix_wrapper/message_queue.hpp b/iceoryx_dust/include/iceoryx_dust/posix_wrapper/message_queue.hpp index a251e650fa5..eda117fa7b5 100644 --- a/iceoryx_dust/include/iceoryx_dust/posix_wrapper/message_queue.hpp +++ b/iceoryx_dust/include/iceoryx_dust/posix_wrapper/message_queue.hpp @@ -56,11 +56,7 @@ class MessageQueue static constexpr uint64_t MAX_MESSAGE_SIZE = 4096; static constexpr uint64_t MAX_MESSAGE_NUMBER = 10; - /// @todo iox-#1036 Remove when all channels are ported to the builder pattern - /// default constructor. The result is an invalid MessageQueue object which can be reassigned later by using the - /// move constructor. - MessageQueue() noexcept = default; - + MessageQueue() noexcept = delete; MessageQueue(const MessageQueue& other) = delete; MessageQueue(MessageQueue&& other) noexcept; MessageQueue& operator=(const MessageQueue& other) = delete; diff --git a/iceoryx_hoofs/include/iceoryx_hoofs/internal/posix_wrapper/unix_domain_socket.hpp b/iceoryx_hoofs/include/iceoryx_hoofs/internal/posix_wrapper/unix_domain_socket.hpp index dd14f3e5716..e91773c0c2d 100644 --- a/iceoryx_hoofs/include/iceoryx_hoofs/internal/posix_wrapper/unix_domain_socket.hpp +++ b/iceoryx_hoofs/include/iceoryx_hoofs/internal/posix_wrapper/unix_domain_socket.hpp @@ -50,11 +50,7 @@ class UnixDomainSocket using result_t = expected; using errorType_t = IpcChannelError; - /// @brief default constructor. The result is an invalid UnixDomainSocket object which can be reassigned later by - /// using the - /// move constructor. - UnixDomainSocket() noexcept = default; - + UnixDomainSocket() noexcept = delete; UnixDomainSocket(const UnixDomainSocket& other) = delete; UnixDomainSocket(UnixDomainSocket&& other) noexcept; UnixDomainSocket& operator=(const UnixDomainSocket& other) = delete; diff --git a/iceoryx_hoofs/test/moduletests/test_unix_domain_sockets.cpp b/iceoryx_hoofs/test/moduletests/test_unix_domain_sockets.cpp index 65c1e29c581..c6af92d0093 100644 --- a/iceoryx_hoofs/test/moduletests/test_unix_domain_sockets.cpp +++ b/iceoryx_hoofs/test/moduletests/test_unix_domain_sockets.cpp @@ -52,15 +52,6 @@ class UnixDomainSocket_test : public Test public: void SetUp() override { - auto serverResult = UnixDomainSocket::create( - goodName, IpcChannelSide::SERVER, UnixDomainSocket::MAX_MESSAGE_SIZE, MaxMsgNumber); - ASSERT_THAT(serverResult.has_error(), Eq(false)); - server = std::move(serverResult.value()); - - auto clientResult = UnixDomainSocket::create( - goodName, IpcChannelSide::CLIENT, UnixDomainSocket::MAX_MESSAGE_SIZE, MaxMsgNumber); - ASSERT_THAT(clientResult.has_error(), Eq(false)); - client = std::move(clientResult.value()); } void TearDown() override @@ -118,8 +109,12 @@ class UnixDomainSocket_test : public Test const std::chrono::milliseconds WAIT_IN_MS{10}; std::atomic_bool doWaitForThread{true}; static constexpr uint64_t MaxMsgNumber = 10U; - UnixDomainSocket server; - UnixDomainSocket client; + UnixDomainSocket server{ + UnixDomainSocket::create(goodName, IpcChannelSide::SERVER, UnixDomainSocket::MAX_MESSAGE_SIZE, MaxMsgNumber) + .expect("Valid UnixDomainSocket")}; + UnixDomainSocket client{ + UnixDomainSocket::create(goodName, IpcChannelSide::CLIENT, UnixDomainSocket::MAX_MESSAGE_SIZE, MaxMsgNumber) + .expect("Valid UnixDomainSocket")}; }; constexpr uint64_t UnixDomainSocket_test::MaxMsgNumber; diff --git a/iceoryx_posh/include/iceoryx_posh/internal/runtime/ipc_interface_base.hpp b/iceoryx_posh/include/iceoryx_posh/internal/runtime/ipc_interface_base.hpp index f0485282015..ee259a089e1 100644 --- a/iceoryx_posh/include/iceoryx_posh/internal/runtime/ipc_interface_base.hpp +++ b/iceoryx_posh/include/iceoryx_posh/internal/runtime/ipc_interface_base.hpp @@ -29,6 +29,7 @@ #include "iceoryx_posh/internal/runtime/ipc_message.hpp" #include "iox/deadline_timer.hpp" #include "iox/duration.hpp" +#include "iox/optional.hpp" #include "iox/relative_pointer.hpp" #include "iceoryx_dust/posix_wrapper/message_queue.hpp" @@ -265,7 +266,7 @@ class IpcInterface uint64_t m_maxMessageSize{0U}; uint64_t m_maxMessages{0U}; iox::posix::IpcChannelSide m_channelSide{posix::IpcChannelSide::CLIENT}; - IpcChannelType m_ipcChannel; + optional m_ipcChannel; }; using IpcInterfaceBase = IpcInterface; diff --git a/iceoryx_posh/source/runtime/ipc_interface_base.cpp b/iceoryx_posh/source/runtime/ipc_interface_base.cpp index 205356a76a1..b4d6ba7f7cd 100644 --- a/iceoryx_posh/source/runtime/ipc_interface_base.cpp +++ b/iceoryx_posh/source/runtime/ipc_interface_base.cpp @@ -78,7 +78,7 @@ IpcInterface::IpcInterface(const RuntimeName_t& runtimeName, template bool IpcInterface::receive(IpcMessage& answer) const noexcept { - auto message = m_ipcChannel.receive(); + auto message = m_ipcChannel->receive(); if (message.has_error()) { return false; @@ -90,7 +90,7 @@ bool IpcInterface::receive(IpcMessage& answer) const noexcept template bool IpcInterface::timedReceive(const units::Duration timeout, IpcMessage& answer) const noexcept { - return !m_ipcChannel.timedReceive(timeout) + return !m_ipcChannel->timedReceive(timeout) .and_then([&answer](auto& message) { IpcInterface::setMessageFromString(message.c_str(), answer); }) @@ -127,7 +127,7 @@ bool IpcInterface::send(const IpcMessage& msg) const noexcept IOX_LOG(ERROR) << "msg size of " << messageSize << " bigger than configured max message size"; } }; - return !m_ipcChannel.send(msg.getMessage()).or_else(logLengthError).has_error(); + return !m_ipcChannel->send(msg.getMessage()).or_else(logLengthError).has_error(); } template @@ -147,7 +147,7 @@ bool IpcInterface::timedSend(const IpcMessage& msg, units::Durat IOX_LOG(ERROR) << "msg size of " << messageSize << " bigger than configured max message size"; } }; - return !m_ipcChannel.timedSend(msg.getMessage(), timeout).or_else(logLengthError).has_error(); + return !m_ipcChannel->timedSend(msg.getMessage(), timeout).or_else(logLengthError).has_error(); } template @@ -159,7 +159,7 @@ const RuntimeName_t& IpcInterface::getRuntimeName() const noexce template bool IpcInterface::isInitialized() const noexcept { - return m_ipcChannel.isInitialized(); + return m_ipcChannel->isInitialized(); } template @@ -167,12 +167,12 @@ bool IpcInterface::openIpcChannel(const posix::IpcChannelSide ch { m_channelSide = channelSide; IpcChannelType::create(m_runtimeName, m_channelSide, m_maxMessageSize, m_maxMessages) - .and_then([this](auto& ipcChannel) { this->m_ipcChannel = std::move(ipcChannel); }) + .and_then([this](auto& ipcChannel) { this->m_ipcChannel.emplace(std::move(ipcChannel)); }) .or_else([](auto& err) { IOX_LOG(ERROR) << "unable to create ipc channel with error code: " << static_cast(err); }); - return m_ipcChannel.isInitialized(); + return m_ipcChannel->isInitialized(); } template @@ -184,7 +184,7 @@ bool IpcInterface::reopen() noexcept template bool IpcInterface::ipcChannelMapsToFile() noexcept { - return !m_ipcChannel.isOutdated().value_or(true); + return !m_ipcChannel->isOutdated().value_or(true); } template <> @@ -202,7 +202,7 @@ bool IpcInterface::ipcChannelMapsToFile() noexcept template bool IpcInterface::hasClosableIpcChannel() const noexcept { - return m_ipcChannel.isInitialized(); + return m_ipcChannel->isInitialized(); } template diff --git a/iceoryx_posh/test/integrationtests/test_mq_interface_startup_race.cpp b/iceoryx_posh/test/integrationtests/test_mq_interface_startup_race.cpp index c5b5338ecf0..2b813f098e4 100644 --- a/iceoryx_posh/test/integrationtests/test_mq_interface_startup_race.cpp +++ b/iceoryx_posh/test/integrationtests/test_mq_interface_startup_race.cpp @@ -59,14 +59,9 @@ class StringToMessage : public IpcInterfaceBase class CMqInterfaceStartupRace_test : public Test { public: - CMqInterfaceStartupRace_test() - : m_appQueue{platform::IoxIpcChannelType::create()} - { - } - virtual void SetUp() { - ASSERT_THAT(m_roudiQueue.has_error(), false); + ASSERT_THAT(m_roudiQueue.has_value(), true); } virtual void TearDown() { @@ -102,11 +97,13 @@ class CMqInterfaceStartupRace_test : public Test regAck << IpcMessageTypeToString(IpcMessageType::REG_ACK) << DUMMY_SHM_SIZE << DUMMY_SHM_OFFSET << oldMsg.getElementAtIndex(INDEX_OF_TIMESTAMP) << DUMMY_SEGMENT_ID << SEND_KEEP_ALIVE; - if (m_appQueue.has_error()) + if (!m_appQueue.has_value()) { - m_appQueue = platform::IoxIpcChannelType::create(MqAppName, IpcChannelSide::CLIENT); + platform::IoxIpcChannelType::create(MqAppName, IpcChannelSide::CLIENT).and_then([this](auto& channel) { + this->m_appQueue.emplace(std::move(channel)); + }); } - ASSERT_THAT(m_appQueue.has_error(), false); + ASSERT_THAT(m_appQueue.has_value(), true); ASSERT_FALSE(m_appQueue->send(regAck.getMessage()).has_error()); } @@ -116,7 +113,7 @@ class CMqInterfaceStartupRace_test : public Test platform::IoxIpcChannelType::result_t m_roudiQueue{ platform::IoxIpcChannelType::create(roudi::IPC_CHANNEL_ROUDI_NAME, IpcChannelSide::SERVER)}; std::mutex m_appQueueMutex; - platform::IoxIpcChannelType::result_t m_appQueue; + optional m_appQueue; }; #if !defined(__APPLE__)