iox-eclipse-iceoryx#381 insert leading slash in MessageQueue

Signed-off-by: Dietrich Krönke <dietrich.kroenke@apex.ai>
ApexAI · Dec 12, 2020 · 5771c49 · 5771c49
1 parent d795540
commit 5771c49
Show file tree

Hide file tree

Showing 4 changed files with 43 additions and 20 deletions.
diff --git a/iceoryx_utils/include/iceoryx_utils/internal/posix_wrapper/message_queue.hpp b/iceoryx_utils/include/iceoryx_utils/internal/posix_wrapper/message_queue.hpp
@@ -104,6 +104,7 @@ class MessageQueue : public DesignPattern::Creation<MessageQueue, IpcChannelErro
     cxx::expected<IpcChannelError> unlink();
     cxx::error<IpcChannelError> createErrorFromErrnum(const int32_t errnum) const;
     static cxx::error<IpcChannelError> createErrorFromErrnum(const ProcessName_t& name, const int32_t errnum);
+    static cxx::expected<ProcessName_t, IpcChannelError> isNameValid(const ProcessName_t& name) noexcept;
 
   private:
     ProcessName_t m_name;

diff --git a/iceoryx_utils/source/posix_wrapper/message_queue.cpp b/iceoryx_utils/source/posix_wrapper/message_queue.cpp
@@ -37,9 +37,15 @@ MessageQueue::MessageQueue(const ProcessName_t& name,
                            const IpcChannelSide channelSide,
                            const size_t maxMsgSize,
                            const uint64_t maxMsgNumber)
-    : m_name{name}
-    , m_channelSide(channelSide)
+    : m_channelSide(channelSide)
 {
+    isNameValid(name)
+        .and_then([this](ProcessName_t& name) { m_name = std::move(name); })
+        .or_else([this](IpcChannelError) {
+            this->m_isInitialized = false;
+            this->m_errorValue = IpcChannelError::INVALID_CHANNEL_NAME;
+        });
+
     if (maxMsgSize > MAX_MESSAGE_SIZE)
     {
         this->m_isInitialized = false;
@@ -55,7 +61,7 @@ MessageQueue::MessageQueue(const ProcessName_t& name,
             {
                 if (mqCall.getErrNum() != ENOENT)
                 {
-                    std::cout << "MQ still there, doing an unlink of " << name << std::endl;
+                    std::cout << "MQ still there, doing an unlink of " << m_name << std::endl;
                 }
             }
         }
@@ -69,7 +75,7 @@ MessageQueue::MessageQueue(const ProcessName_t& name,
         m_attributes.mq_recvwait = 0;
         m_attributes.mq_sendwait = 0;
 #endif
-        auto openResult = open(name, mode, channelSide);
+        auto openResult = open(m_name, mode, channelSide);
 
         if (!openResult.has_error())
         {
@@ -121,13 +127,14 @@ MessageQueue& MessageQueue::operator=(MessageQueue&& other)
 
 cxx::expected<bool, IpcChannelError> MessageQueue::unlinkIfExists(const ProcessName_t& name)
 {
-    if (name.size() < SHORTEST_VALID_QUEUE_NAME || name.c_str()[0] != '/')
+    ProcessName_t l_name;
+    if (isNameValid(name).and_then([&](ProcessName_t& name) { l_name = std::move(name); }).has_error())
     {
         return cxx::error<IpcChannelError>(IpcChannelError::INVALID_CHANNEL_NAME);
     }
 
     auto mqCall =
-        cxx::makeSmartC(mq_unlink, cxx::ReturnMode::PRE_DEFINED_ERROR_CODE, {ERROR_CODE}, {ENOENT}, name.c_str());
+        cxx::makeSmartC(mq_unlink, cxx::ReturnMode::PRE_DEFINED_ERROR_CODE, {ERROR_CODE}, {ENOENT}, l_name.c_str());
 
     if (!mqCall.hasErrors())
     {
@@ -136,7 +143,7 @@ cxx::expected<bool, IpcChannelError> MessageQueue::unlinkIfExists(const ProcessN
     }
     else
     {
-        return createErrorFromErrnum(name, mqCall.getErrNum());
+        return createErrorFromErrnum(l_name, mqCall.getErrNum());
     }
 }
 
@@ -211,7 +218,8 @@ cxx::expected<std::string, IpcChannelError> MessageQueue::receive() const
 cxx::expected<int32_t, IpcChannelError>
 MessageQueue::open(const ProcessName_t& name, const IpcChannelMode mode, const IpcChannelSide channelSide)
 {
-    if (name.size() < SHORTEST_VALID_QUEUE_NAME || name.c_str()[0] != '/')
+    ProcessName_t l_name;
+    if (isNameValid(name).and_then([&](ProcessName_t& name) { l_name = std::move(name); }).has_error())
     {
         return cxx::error<IpcChannelError>(IpcChannelError::INVALID_CHANNEL_NAME);
     }
@@ -230,7 +238,7 @@ MessageQueue::open(const ProcessName_t& name, const IpcChannelMode mode, const I
                                   cxx::ReturnMode::PRE_DEFINED_ERROR_CODE,
                                   {ERROR_CODE},
                                   {ENOENT},
-                                  name.c_str(),
+                                  l_name.c_str(),
                                   openFlags,
                                   m_filemode,
                                   &m_attributes);
@@ -414,5 +422,26 @@ cxx::error<IpcChannelError> MessageQueue::createErrorFromErrnum(const ProcessNam
     }
 }
 
+cxx::expected<ProcessName_t, IpcChannelError> MessageQueue::isNameValid(const ProcessName_t& name) noexcept
+{
+    /// @todo the check for the longest valid queue name is missing
+    /// the name for the mqeue is limited by MAX_PATH
+    /// The mq_open call is wrapped by smartC to throw then an ENAMETOOLONG error
+    /// See: https://pubs.opengroup.org/onlinepubs/9699919799/functions/mq_open.html
+    if (name.empty() || name.size() < SHORTEST_VALID_QUEUE_NAME)
+    {
+        return cxx::error<IpcChannelError>(IpcChannelError::INVALID_CHANNEL_NAME);
+    }
+    else if (name.c_str()[0] != '/')
+    {
+        std::clog << "Adding leading slash in name for Message Queue. Name is maybe truncated." << std::endl;
+        return cxx::success<ProcessName_t>(ProcessName_t("/").append(iox::cxx::TruncateToCapacity, name));
+    }
+    else
+    {
+        return cxx::success<ProcessName_t>(name);
+    }
+}
+
 } // namespace posix
 } // namespace iox
diff --git a/iceoryx_utils/source/posix_wrapper/unix_domain_socket.cpp b/iceoryx_utils/source/posix_wrapper/unix_domain_socket.cpp
@@ -535,8 +535,7 @@ cxx::error<IpcChannelError> UnixDomainSocket::createErrorFromErrnum(const int32_
 
 bool UnixDomainSocket::isNameValid(const ProcessName_t& name) noexcept
 {
-    return !(name.empty() || name.size() < SHORTEST_VALID_NAME || name.size() > LONGEST_VALID_NAME
-             || name.c_str()[0] != '/');
+    return !(name.empty() || name.size() < SHORTEST_VALID_NAME || name.size() > LONGEST_VALID_NAME);
 }
 
 

diff --git a/iceoryx_utils/test/moduletests/test_ipc_channel.cpp b/iceoryx_utils/test/moduletests/test_ipc_channel.cpp
@@ -30,9 +30,9 @@ using IpcChannelTypes = Types<UnixDomainSocket>;
 using IpcChannelTypes = Types<MessageQueue, UnixDomainSocket>;
 #endif
 
-constexpr char goodName[] = "/channel_test";
-constexpr char anotherGoodName[] = "/horst";
-constexpr char theUnknown[] = "/WhoeverYouAre";
+constexpr char goodName[] = "channel_test";
+constexpr char anotherGoodName[] = "horst";
+constexpr char theUnknown[] = "WhoeverYouAre";
 constexpr char badName[] = "skdhnskähug";
 
 template <typename T>
@@ -92,12 +92,6 @@ TYPED_TEST(IpcChannel_test, createNoName)
     ASSERT_THAT(serverResult.get_error(), Eq(IpcChannelError::INVALID_CHANNEL_NAME));
 }
 
-TYPED_TEST(IpcChannel_test, createBadName)
-{
-    auto serverResult = TestFixture::IpcChannelType::create(badName, IpcChannelMode::BLOCKING, IpcChannelSide::SERVER);
-    EXPECT_TRUE(serverResult.has_error());
-}
-
 TYPED_TEST(IpcChannel_test, createAgain)
 {
     // if there is a leftover from a crashed channel, we can create a new one. This is simulated by creating twice