fix(io): memory_mapped_file is move-only

include/cpp_utils/io/memory_mapped_file.hpp

@@ -84,8 +84,8 @@ struct memory_mapped_file
 #ifdef USE_MMAP
                 if (fd = open(path.c_str(), O_RDONLY, static_cast<mode_t>(0600)); fd != -1)
                 {
-                    mapped_file = details::checked_mmap(mmap(
-                        nullptr, this->f_size, PROT_READ, MAP_FILE | MAP_PRIVATE, fd, 0UL));
+                    mapped_file = details::checked_mmap(
+                        mmap(nullptr, this->f_size, PROT_READ, MAP_FILE | MAP_PRIVATE, fd, 0UL));
                     if (mapped_file == nullptr)
                     {
                         close(fd);
@@ -109,22 +109,25 @@ struct memory_mapped_file
             }
         }
     }
-    ~memory_mapped_file()
+    // A mapping owns a file descriptor and an address range (or Windows handles): a copy would
+    // unmap twice, so copies are deleted and only ownership transfer is allowed.
+    memory_mapped_file(const memory_mapped_file&) = delete;
+    memory_mapped_file& operator=(const memory_mapped_file&) = delete;
+
+    memory_mapped_file(memory_mapped_file&& other) noexcept { adopt(other); }
+
+    memory_mapped_file& operator=(memory_mapped_file&& other) noexcept
     {
-        if (mapped_file)
+        if (this != &other)
         {
-#ifdef USE_MMAP
-            munmap(mapped_file, f_size);
-            close(fd);
-#endif
-#ifdef USE_MapViewOfFile
-            ::UnmapViewOfFile(mapped_file);
-            ::CloseHandle(hFile);
-            ::CloseHandle(hMapFile);
-#endif
+            release();
+            adopt(other);
         }
+        return *this;
     }
 
+    ~memory_mapped_file() { release(); }
+
     auto read(const std::size_t offset, const std::size_t) const { return mapped_file + offset; }
 
     template <std::size_t size>
@@ -167,6 +170,52 @@ struct memory_mapped_file
         return hFile != NULL && hMapFile != NULL && mapped_file != nullptr;
 #endif
     }
+
+private:
+    void release() noexcept
+    {
+        if (mapped_file)
+        {
+#ifdef USE_MMAP
+            munmap(mapped_file, f_size);
+            close(fd);
+#endif
+#ifdef USE_MapViewOfFile
+            ::UnmapViewOfFile(mapped_file);
+            ::CloseHandle(hFile);
+            ::CloseHandle(hMapFile);
+#endif
+        }
+        reset();
+    }
+
+    void adopt(memory_mapped_file& other) noexcept
+    {
+#ifdef USE_MMAP
+        fd = other.fd;
+#endif
+#ifdef USE_MapViewOfFile
+        hMapFile = other.hMapFile;
+        hFile = other.hFile;
+#endif
+        mapped_file = other.mapped_file;
+        f_size = other.f_size;
+        other.reset();
+    }
+
+    // Leaves the object owning nothing, so its destructor is a no-op.
+    void reset() noexcept
+    {
+#ifdef USE_MMAP
+        fd = -1;
+#endif
+#ifdef USE_MapViewOfFile
+        hMapFile = NULL;
+        hFile = NULL;
+#endif
+        mapped_file = nullptr;
+        f_size = 0UL;
+    }
 };
 
 static_assert(std::ranges::contiguous_range<memory_mapped_file>);

tests/io/test_memory_mapped_file.cpp

@@ -156,3 +156,67 @@ TEST_CASE("memory_mapped_file cleans up its fd when mmap() itself fails", "[io]"
     std::filesystem::remove(path);
 }
 #endif
+
+TEST_CASE("memory_mapped_file survives a move and unmaps only once", "[io]")
+{
+    auto path = std::filesystem::temp_directory_path() / "cpp_utils_mmf_move_test.txt";
+    {
+        std::ofstream out(path, std::ios::binary);
+        out << "hello world";
+    }
+
+    // The moved-from object dies before the data is read: if it still owned the mapping its
+    // destructor would unmap the bytes the moved-to object is about to read.
+    auto moved = [&path]
+    {
+        memory_mapped_file source { path.string() };
+        REQUIRE(source.is_valid());
+        memory_mapped_file target { std::move(source) };
+        REQUIRE_FALSE(source.is_valid());
+        REQUIRE(source.data() == nullptr);
+        REQUIRE(source.size() == 0);
+        return target;
+    }();
+
+    REQUIRE(moved.is_valid());
+    REQUIRE(moved.size() == 11);
+    REQUIRE(std::string(moved.data(), moved.size()) == "hello world");
+
+    std::filesystem::remove(path);
+}
+
+TEST_CASE("memory_mapped_file move assignment releases the previous mapping", "[io]")
+{
+    auto first = std::filesystem::temp_directory_path() / "cpp_utils_mmf_move_a.txt";
+    auto second = std::filesystem::temp_directory_path() / "cpp_utils_mmf_move_b.txt";
+    {
+        std::ofstream out(first, std::ios::binary);
+        out << "first";
+    }
+    {
+        std::ofstream out(second, std::ios::binary);
+        out << "second payload";
+    }
+
+    memory_mapped_file target { first.string() };
+    REQUIRE(std::string(target.data(), target.size()) == "first");
+
+    {
+        memory_mapped_file source { second.string() };
+        target = std::move(source);
+        REQUIRE_FALSE(source.is_valid());
+    }
+
+    REQUIRE(target.is_valid());
+    REQUIRE(target.size() == 14);
+    REQUIRE(std::string(target.data(), target.size()) == "second payload");
+
+    std::filesystem::remove(first);
+    std::filesystem::remove(second);
+}
+
+// Owning a raw mapping/handle means copies would unmap twice; CEFast already hit this.
+static_assert(!std::is_copy_constructible_v<memory_mapped_file>);
+static_assert(!std::is_copy_assignable_v<memory_mapped_file>);
+static_assert(std::is_move_constructible_v<memory_mapped_file>);
+static_assert(std::is_move_assignable_v<memory_mapped_file>);