[Minor] More tests and fixes to raii file

diff --git a/src/libutil/cxx/locked_file.cxx b/src/libutil/cxx/locked_file.cxx
index 4c91f44ed560a4a6e818a9386f788b7a58963f64..c972e1de30faecb465bb103533514eb5ae7baa13 100644 (file)
--- a/src/libutil/cxx/locked_file.cxx
+++ b/src/libutil/cxx/locked_file.cxx
@@ -41,11 +41,6 @@ auto raii_file::open(const char *fname, int flags) -> tl::expected<raii_file, st
                return tl::make_unexpected(fmt::format("cannot open file {}: {}", fname, ::strerror(errno)));
        }
 
-       if (!rspamd_file_lock(fd, TRUE)) {
-               close(fd);
-               return tl::make_unexpected(fmt::format("cannot lock file {}: {}", fname, ::strerror(errno)));
-       }
-
        auto ret = raii_file{fname, fd, false};
 
        if (fstat(ret.fd, &ret.st) == -1) {
@@ -72,11 +67,6 @@ auto raii_file::create(const char *fname, int flags, int perms) -> tl::expected<
                return tl::make_unexpected(fmt::format("cannot create file {}: {}", fname, ::strerror(errno)));
        }
 
-       if (!rspamd_file_lock(fd, TRUE)) {
-               close(fd);
-               return tl::make_unexpected(fmt::format("cannot lock file {}: {}", fname, ::strerror(errno)));
-       }
-
        auto ret = raii_file{fname, fd, false};
 
        if (fstat(ret.fd, &ret.st) == -1) {
@@ -102,12 +92,6 @@ auto raii_file::create_temp(const char *fname, int flags, int perms) -> tl::expe
                return tl::make_unexpected(fmt::format("cannot create file {}: {}", fname, ::strerror(errno)));
        }
 
-       if (!rspamd_file_lock(fd, TRUE)) {
-               unlink(fname);
-               close(fd);
-               return tl::make_unexpected(fmt::format("cannot lock file {}: {}", fname, ::strerror(errno)));
-       }
-
        auto ret = raii_file{fname, fd, true};
 
        if (fstat(ret.fd, &ret.st) == -1) {
@@ -135,12 +119,6 @@ auto raii_file::mkstemp(const char *pattern, int flags, int perms) -> tl::expect
                return tl::make_unexpected(fmt::format("cannot create file {}: {}", pattern, ::strerror(errno)));
        }
 
-       if (!rspamd_file_lock(fd, TRUE)) {
-               close(fd);
-               (void)unlink(mutable_pattern.c_str());
-               return tl::make_unexpected(fmt::format("cannot lock file {}: {}", pattern, ::strerror(errno)));
-       }
-
        auto ret = raii_file{mutable_pattern.c_str(), fd, true};
 
        if (fstat(ret.fd, &ret.st) == -1) {
@@ -157,11 +135,15 @@ raii_file::~raii_file() noexcept
                if (temp) {
                        (void)unlink(fname.c_str());
                }
-               (void) rspamd_file_unlock(fd, FALSE);
                close(fd);
        }
 }
 
+auto raii_file::update_stat() noexcept -> bool
+{
+       return fstat(fd, &st) != -1;
+}
+
 
 raii_locked_file::~raii_locked_file() noexcept
 {
@@ -197,6 +179,8 @@ auto raii_mmaped_file::mmap_shared(raii_file &&file,
 {
        void *map;
 
+       /* Update stat on file to ensure it is up-to-date */
+       file.update_stat();
        map = mmap(NULL, file.get_stat().st_size, flags, MAP_SHARED, file.get_fd(), 0);
 
        if (map == MAP_FAILED) {
@@ -386,6 +370,36 @@ TEST_CASE("tempfile") {
        CHECK(serrno == ENOENT);
 }
 
+TEST_CASE("mmap") {
+       std::string tmpname;
+       {
+               auto raii_file = raii_file::mkstemp("/tmp//doctest-XXXXXXXX",
+               O_RDWR|O_CREAT|O_EXCL, 00600);
+               CHECK(raii_file.has_value());
+               CHECK(raii_file->get_dir() == "/tmp");
+               CHECK(access(raii_file->get_name().data(), R_OK) == 0);
+               tmpname = std::string{raii_file->get_name()};
+               char payload[] = {'1', '2', '3'};
+               CHECK(write(raii_file->get_fd(), payload, sizeof(payload)) == sizeof(payload));
+               auto mmapped_file1 = raii_mmaped_file::mmap_shared(std::move(raii_file.value()), PROT_READ|PROT_WRITE);
+               CHECK(mmapped_file1.has_value());
+               CHECK(!raii_file->is_valid());
+               CHECK(mmapped_file1->get_size() == sizeof(payload));
+               CHECK(memcmp(mmapped_file1->get_map(), payload, sizeof(payload)) == 0);
+               *(char *)mmapped_file1->get_map() = '2';
+               auto mmapped_file2 = raii_mmaped_file::mmap_shared(tmpname.c_str(), O_RDONLY, PROT_READ);
+               CHECK(mmapped_file2.has_value());
+               CHECK(mmapped_file2->get_size() == sizeof(payload));
+               CHECK(memcmp(mmapped_file2->get_map(), payload, sizeof(payload)) != 0);
+               CHECK(memcmp(mmapped_file2->get_map(), mmapped_file1->get_map(), sizeof(payload)) == 0);
+       }
+       // File must be deleted after this call
+       auto ret = ::access(tmpname.c_str(), R_OK);
+       auto serrno = errno;
+       CHECK(ret == -1);
+       CHECK(serrno == ENOENT);
+}
+
 } // TEST_SUITE
 
 } // namespace tests

diff --git a/src/libutil/cxx/locked_file.hxx b/src/libutil/cxx/locked_file.hxx
index c7d286cbb725e2c19c3576777286afa4c721a7a3..507b02762dd95750602436e5d60eb0abad9782f7 100644 (file)
--- a/src/libutil/cxx/locked_file.hxx
+++ b/src/libutil/cxx/locked_file.hxx
@@ -44,6 +44,10 @@ public:
                return st;
        };
 
+       auto get_size() const -> std::size_t {
+               return st.st_size;
+       };
+
        auto get_name() const -> std::string_view {
                return std::string_view{fname};
        }
@@ -93,10 +97,24 @@ public:
                *this = std::move(other);
        }
 
+       /**
+        * Prevent file from being deleted
+        * @return
+        */
        auto make_immortal() noexcept {
                temp = false;
        }
 
+       /**
+        * Performs fstat on an opened file to refresh internal stat
+        * @return
+        */
+       auto update_stat() noexcept -> bool;
+
+       auto is_valid() noexcept -> bool {
+               return fd != -1;
+       }
+
        /* Do not allow copy/default ctor */
        const raii_file& operator=(const raii_file &other) = delete;
        raii_file() = delete;
@@ -181,6 +199,7 @@ struct raii_mmaped_file final {
        static auto mmap_shared(const char *fname, int open_flags, int mmap_flags) -> tl::expected<raii_mmaped_file, std::string>;
        // Returns a constant pointer to the underlying map
        auto get_map() const -> void* {return map;}
+       auto get_file() const -> const raii_file& { return file; }
        // Passes the ownership of the mmaped memory to the callee
        auto steal_map() -> std::tuple<void *, std::size_t> {
                auto ret = std::make_tuple(this->map, file.get_stat().st_size);