iox-eclipse-iceoryx#1821 Check for incompatible flags in SharedMemory…

…Builder Signed-off-by: Tobias Stark <[email protected]>
tobiasblass · Jan 31, 2023 · ee3d5a9 · ee3d5a9
1 parent db5d5b2
commit ee3d5a9
Show file tree

Hide file tree

Showing 4 changed files with 34 additions and 10 deletions.
diff --git a/doc/website/release-notes/iceoryx-unreleased.md b/doc/website/release-notes/iceoryx-unreleased.md
@@ -64,6 +64,8 @@
 - Fix double move in `vector::emplace` [\#1823](https://github.com/eclipse-iceoryx/iceoryx/issues/1823)
 - Default roudi_config.toml path is not used [\#1826](https://github.com/eclipse-iceoryx/iceoryx/issues/1826)
 - `WaitSet::wait` returns if data was send before `WaitSet::attachState(.., State::HAS_{DATA, REQUEST, RESPONSE})` [\#1855](https://github.com/eclipse-iceoryx/iceoryx/issues/1855)
+- Provide a better error message when attempting to create a shared memory in read-only mode
+  [\#1821](https://github.com/eclipse-iceoryx/iceoryx/issues/1821)
 
 **Refactoring:**
 

diff --git a/...hoofs/include/iceoryx_hoofs/internal/posix_wrapper/shared_memory_object/shared_memory.hpp b/...hoofs/include/iceoryx_hoofs/internal/posix_wrapper/shared_memory_object/shared_memory.hpp
@@ -47,6 +47,7 @@ enum class SharedMemoryError
     NO_FILE_RESIZE_SUPPORT,
     NO_RESIZE_SUPPORT,
     INVALID_FILEDESCRIPTOR,
+    INCOMPATIBLE_OPEN_AND_ACCESS_MODE,
     UNKNOWN_ERROR
 };
 

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
@@ -68,6 +68,16 @@ expected<SharedMemory, SharedMemoryError> SharedMemoryBuilder::create() noexcept
 
     auto nameWithLeadingSlash = addLeadingSlash(m_name);
 
+    bool hasOwnership = (m_openMode == OpenMode::EXCLUSIVE_CREATE || m_openMode == OpenMode::PURGE_AND_CREATE
+                         || m_openMode == OpenMode::OPEN_OR_CREATE);
+
+    if (hasOwnership && (m_accessMode == AccessMode::READ_ONLY))
+    {
+        std::cerr << "Cannot create shared-memory file \"" << m_name << "\" in read-only mode. "
+                  << "Initializing a new file requires write access" << std::endl;
+        return cxx::error<SharedMemoryError>(SharedMemoryError::INCOMPATIBLE_OPEN_AND_ACCESS_MODE);
+    }
+
     // 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);
@@ -97,27 +107,24 @@ expected<SharedMemory, SharedMemoryError> SharedMemoryBuilder::create() noexcept
             // ownership and we just try to open it
             if (m_openMode == OpenMode::OPEN_OR_CREATE && result.get_error().errnum == EEXIST)
             {
+                hasOwnership = false;
                 result = posixCall(iox_shm_open)(nameWithLeadingSlash.c_str(),
                                                  convertToOflags(m_accessMode, OpenMode::OPEN_EXISTING),
                                                  static_cast<mode_t>(m_filePermissions))
                              .failureReturnValue(SharedMemory::INVALID_HANDLE)
                              .evaluate();
-                if (!result.has_error())
-                {
-                    constexpr bool HAS_NO_OWNERSHIP = false;
-                    sharedMemoryFileHandle = result->value;
-                    return success<SharedMemory>(SharedMemory(m_name, sharedMemoryFileHandle, HAS_NO_OWNERSHIP));
-                }
             }
 
-            printError();
-            return error<SharedMemoryError>(SharedMemory::errnoToEnum(result.get_error().errnum));
+            // Check again, as the if-block above may have changed `result`
+            if (result.has_error())
+            {
+                printError();
+                return error<SharedMemoryError>(SharedMemory::errnoToEnum(result.get_error().errnum));
+            }
         }
         sharedMemoryFileHandle = result->value;
     }
 
-    const bool hasOwnership = (m_openMode == OpenMode::EXCLUSIVE_CREATE || m_openMode == OpenMode::PURGE_AND_CREATE
-                               || m_openMode == OpenMode::OPEN_OR_CREATE);
     if (hasOwnership)
     {
         auto result = posixCall(ftruncate)(sharedMemoryFileHandle, static_cast<int64_t>(m_size))

diff --git a/iceoryx_hoofs/test/moduletests/test_posix_shared_memory.cpp b/iceoryx_hoofs/test/moduletests/test_posix_shared_memory.cpp
@@ -250,5 +250,19 @@ TEST_F(SharedMemory_Test, OpenFailsWhenShmDoesNotExist)
     ASSERT_TRUE(sut.has_error());
 }
 
+TEST_F(SharedMemory_Test, OpenFailsWhenCreatingShmInReadOnlyMode)
+{
+    ::testing::Test::RecordProperty("TEST_ID", "80684160-b243-4ca1-b285-118d2ef36108");
+    auto sut = iox::posix::SharedMemoryBuilder()
+                   .name("readOnlyShmMem")
+                   .size(100)
+                   .accessMode(iox::posix::AccessMode::READ_ONLY)
+                   .openMode(iox::posix::OpenMode::PURGE_AND_CREATE)
+                   .create();
+
+    ASSERT_TRUE(sut.has_error());
+    ASSERT_THAT(sut.get_error(), Eq(SharedMemoryError::INCOMPATIBLE_OPEN_AND_ACCESS_MODE));
+}
+
 
 } // namespace