[Rework] Split locked and unlocked files, as mmap does not need flock normally

diff --git a/src/libutil/cxx/locked_file.cxx b/src/libutil/cxx/locked_file.cxx
index bd25b0fc7798bfd352235171ee5044a15b13c0e1..b4d865626cafa6de2f45a2f0e270a149a8e5c84b 100644 (file)
--- a/src/libutil/cxx/locked_file.cxx
+++ b/src/libutil/cxx/locked_file.cxx
@@ -24,7 +24,7 @@
 
 namespace rspamd::util {
 
-auto raii_locked_file::open(const char *fname, int flags) -> tl::expected<raii_locked_file, std::string>
+auto raii_file::open(const char *fname, int flags) -> tl::expected<raii_file, std::string>
 {
        int oflags = flags;
 #ifdef O_CLOEXEC
@@ -46,7 +46,7 @@ auto raii_locked_file::open(const char *fname, int flags) -> tl::expected<raii_l
                return tl::make_unexpected(fmt::format("cannot lock file {}: {}", fname, ::strerror(errno)));
        }
 
-       auto ret = raii_locked_file{fname, fd, false};
+       auto ret = raii_file{fname, fd, false};
 
        if (fstat(ret.fd, &ret.st) == -1) {
                return tl::make_unexpected(fmt::format("cannot stat file {}: {}", fname, ::strerror(errno)));
@@ -55,18 +55,7 @@ auto raii_locked_file::open(const char *fname, int flags) -> tl::expected<raii_l
        return ret;
 }
 
-raii_locked_file::~raii_locked_file()
-{
-       if (fd != -1) {
-               if (temp) {
-                       (void)unlink(fname.c_str());
-               }
-               (void) rspamd_file_unlock(fd, FALSE);
-               close(fd);
-       }
-}
-
-auto raii_locked_file::create(const char *fname, int flags, int perms) -> tl::expected<raii_locked_file, std::string>
+auto raii_file::create(const char *fname, int flags, int perms) -> tl::expected<raii_file, std::string>
 {
        int oflags = flags;
 #ifdef O_CLOEXEC
@@ -88,7 +77,7 @@ auto raii_locked_file::create(const char *fname, int flags, int perms) -> tl::ex
                return tl::make_unexpected(fmt::format("cannot lock file {}: {}", fname, ::strerror(errno)));
        }
 
-       auto ret = raii_locked_file{fname, fd, false};
+       auto ret = raii_file{fname, fd, false};
 
        if (fstat(ret.fd, &ret.st) == -1) {
                return tl::make_unexpected(fmt::format("cannot stat file {}: {}", fname, ::strerror(errno)));
@@ -97,7 +86,7 @@ auto raii_locked_file::create(const char *fname, int flags, int perms) -> tl::ex
        return ret;
 }
 
-auto raii_locked_file::create_temp(const char *fname, int flags, int perms) -> tl::expected<raii_locked_file, std::string>
+auto raii_file::create_temp(const char *fname, int flags, int perms) -> tl::expected<raii_file, std::string>
 {
        int oflags = flags;
 #ifdef O_CLOEXEC
@@ -114,11 +103,12 @@ auto raii_locked_file::create_temp(const char *fname, int flags, int perms) -> t
        }
 
        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_locked_file{fname, fd, true};
+       auto ret = raii_file{fname, fd, true};
 
        if (fstat(ret.fd, &ret.st) == -1) {
                return tl::make_unexpected(fmt::format("cannot stat file {}: {}", fname, ::strerror(errno)));
@@ -127,7 +117,7 @@ auto raii_locked_file::create_temp(const char *fname, int flags, int perms) -> t
        return ret;
 }
 
-auto raii_locked_file::mkstemp(const char *pattern, int flags, int perms) -> tl::expected<raii_locked_file, std::string>
+auto raii_file::mkstemp(const char *pattern, int flags, int perms) -> tl::expected<raii_file, std::string>
 {
        int oflags = flags;
 #ifdef O_CLOEXEC
@@ -151,7 +141,7 @@ auto raii_locked_file::mkstemp(const char *pattern, int flags, int perms) -> tl:
                return tl::make_unexpected(fmt::format("cannot lock file {}: {}", pattern, ::strerror(errno)));
        }
 
-       auto ret = raii_locked_file{mutable_pattern.c_str(), fd, true};
+       auto ret = raii_file{mutable_pattern.c_str(), fd, true};
 
        if (fstat(ret.fd, &ret.st) == -1) {
                return tl::make_unexpected(fmt::format("cannot stat file {}: {}",
@@ -161,13 +151,41 @@ auto raii_locked_file::mkstemp(const char *pattern, int flags, int perms) -> tl:
        return ret;
 }
 
-raii_mmaped_locked_file::raii_mmaped_locked_file(raii_locked_file &&_file, void *_map)
+raii_file::~raii_file() noexcept
+{
+       if (fd != -1) {
+               if (temp) {
+                       (void)unlink(fname.c_str());
+               }
+               (void) rspamd_file_unlock(fd, FALSE);
+               close(fd);
+       }
+}
+
+
+raii_locked_file::~raii_locked_file() noexcept
+{
+       if (fd != -1) {
+               (void) rspamd_file_unlock(fd, FALSE);
+       }
+}
+
+auto raii_locked_file::lock_raii_file(raii_file &&unlocked) -> tl::expected<raii_locked_file, std::string>
+{
+       if (!rspamd_file_lock(unlocked.get_fd(), TRUE)) {
+               return tl::make_unexpected(fmt::format("cannot lock file {}: {}", unlocked.get_name(), ::strerror(errno)));
+       }
+
+       return raii_locked_file{std::move(unlocked)};
+}
+
+raii_mmaped_file::raii_mmaped_file(raii_file &&_file, void *_map)
                : file(std::move(_file)), map(_map)
 {
 }
 
-auto raii_mmaped_locked_file::mmap_shared(raii_locked_file &&file,
-                                                                                 int flags) -> tl::expected<raii_mmaped_locked_file, std::string>
+auto raii_mmaped_file::mmap_shared(raii_file &&file,
+                                                                  int flags) -> tl::expected<raii_mmaped_file, std::string>
 {
        void *map;
 
@@ -179,29 +197,29 @@ auto raii_mmaped_locked_file::mmap_shared(raii_locked_file &&file,
 
        }
 
-       return raii_mmaped_locked_file{std::move(file), map};
+       return raii_mmaped_file{std::move(file), map};
 }
 
-auto raii_mmaped_locked_file::mmap_shared(const char *fname, int open_flags,
-                                                                                 int mmap_flags) -> tl::expected<raii_mmaped_locked_file, std::string>
+auto raii_mmaped_file::mmap_shared(const char *fname, int open_flags,
+                                                                  int mmap_flags) -> tl::expected<raii_mmaped_file, std::string>
 {
-       auto file = raii_locked_file::open(fname, open_flags);
+       auto file = raii_file::open(fname, open_flags);
 
        if (!file.has_value()) {
                return tl::make_unexpected(file.error());
        }
 
-       return raii_mmaped_locked_file::mmap_shared(std::move(file.value()), mmap_flags);
+       return raii_mmaped_file::mmap_shared(std::move(file.value()), mmap_flags);
 }
 
-raii_mmaped_locked_file::~raii_mmaped_locked_file()
+raii_mmaped_file::~raii_mmaped_file()
 {
        if (map != nullptr) {
                munmap(map, file.get_stat().st_size);
        }
 }
 
-raii_mmaped_locked_file::raii_mmaped_locked_file(raii_mmaped_locked_file &&other) noexcept
+raii_mmaped_file::raii_mmaped_file(raii_mmaped_file &&other) noexcept
                : file(std::move(other.file))
 {
        std::swap(map, other.map);

diff --git a/src/libutil/cxx/locked_file.hxx b/src/libutil/cxx/locked_file.hxx
index 94561a2d45ee757d6880ab2170d89958cb22d625..8690cfb64c685acc2e0c5055579dc8ea1bce3534 100644 (file)
--- a/src/libutil/cxx/locked_file.hxx
+++ b/src/libutil/cxx/locked_file.hxx
@@ -24,16 +24,16 @@
 
 namespace rspamd::util {
 /**
- * A simple RAII object to contain a file descriptor with an flock wrap
+ * A simple RAII object to contain a move only file descriptor
  * A file is unlocked and closed when not needed
  */
-struct raii_locked_file final {
-       ~raii_locked_file();
+struct raii_file {
+       virtual ~raii_file() noexcept;
 
-       static auto open(const char *fname, int flags) -> tl::expected<raii_locked_file, std::string>;
-       static auto create(const char *fname, int flags, int perms) -> tl::expected<raii_locked_file, std::string>;
-       static auto create_temp(const char *fname, int flags, int perms) -> tl::expected<raii_locked_file, std::string>;
-       static auto mkstemp(const char *pattern, int flags, int perms) -> tl::expected<raii_locked_file, std::string>;
+       static auto open(const char *fname, int flags) -> tl::expected<raii_file, std::string>;
+       static auto create(const char *fname, int flags, int perms) -> tl::expected<raii_file, std::string>;
+       static auto create_temp(const char *fname, int flags, int perms) -> tl::expected<raii_file, std::string>;
+       static auto mkstemp(const char *pattern, int flags, int perms) -> tl::expected<raii_file, std::string>;
 
        auto get_fd() const -> int {
                return fd;
@@ -79,7 +79,7 @@ struct raii_locked_file final {
                }
        }
 
-       raii_locked_file& operator=(raii_locked_file &&other) noexcept {
+       raii_file& operator=(raii_file &&other) noexcept {
                std::swap(fd, other.fd);
                std::swap(temp, other.temp);
                std::swap(fname, other.fname);
@@ -88,30 +88,85 @@ struct raii_locked_file final {
                return *this;
        }
 
-       raii_locked_file(raii_locked_file &&other) noexcept {
+       raii_file(raii_file &&other) noexcept {
                *this = std::move(other);
        }
 
        /* Do not allow copy/default ctor */
-       const raii_locked_file& operator=(const raii_locked_file &other) = delete;
-       raii_locked_file() = delete;
-       raii_locked_file(const raii_locked_file &other) = delete;
-private:
+       const raii_file& operator=(const raii_file &other) = delete;
+       raii_file() = delete;
+       raii_file(const raii_file &other) = delete;
+protected:
        int fd = -1;
        bool temp;
        std::string fname;
        struct stat st;
 
-       explicit raii_locked_file(const char *_fname, int _fd, bool _temp) : fd(_fd), temp(_temp), fname(_fname) {}
+       explicit raii_file(const char *fname, int fd, bool temp) : fd(fd), temp(temp), fname(fname) {}
+};
+/**
+ * A simple RAII object to contain a file descriptor with an flock wrap
+ * A file is unlocked and closed when not needed
+ */
+struct raii_locked_file final : public raii_file {
+       ~raii_locked_file() noexcept override;
+
+       static auto open(const char *fname, int flags) -> tl::expected<raii_locked_file, std::string> {
+               auto locked = raii_file::open(fname, flags).and_then([]<class T>(T &&file) {
+                       return lock_raii_file(std::forward<T>(file));
+               });
+
+               return locked;
+       }
+       static auto create(const char *fname, int flags, int perms) -> tl::expected<raii_locked_file, std::string> {
+               auto locked = raii_file::create(fname, flags, perms).and_then([]<class T>(T &&file) {
+                       return lock_raii_file(std::forward<T>(file));
+               });
+
+               return locked;
+       }
+       static auto create_temp(const char *fname, int flags, int perms) -> tl::expected<raii_locked_file, std::string> {
+               auto locked = raii_file::create_temp(fname, flags, perms).and_then([]<class T>(T &&file) {
+                       return lock_raii_file(std::forward<T>(file));
+               });
+
+               return locked;
+       }
+       static auto mkstemp(const char *pattern, int flags, int perms) -> tl::expected<raii_locked_file, std::string> {
+               auto locked = raii_file::mkstemp(pattern, flags, perms).and_then([]<class T>(T &&file) {
+                       return lock_raii_file(std::forward<T>(file));
+               });
+
+               return locked;
+       }
+
+       raii_locked_file& operator=(raii_locked_file &&other) noexcept {
+               std::swap(fd, other.fd);
+               std::swap(temp, other.temp);
+               std::swap(fname, other.fname);
+               std::swap(st, other.st);
+
+               return *this;
+       }
+
+       raii_locked_file(raii_locked_file &&other) noexcept : raii_file(static_cast<raii_file &&>(std::move(other))) {}
+       /* Do not allow copy/default ctor */
+       const raii_locked_file& operator=(const raii_locked_file &other) = delete;
+       raii_locked_file() = delete;
+       raii_locked_file(const raii_locked_file &other) = delete;
+private:
+       static auto lock_raii_file(raii_file &&unlocked) -> tl::expected<raii_locked_file, std::string>;
+       raii_locked_file(raii_file &&other) noexcept : raii_file(std::move(other)) {}
+       explicit raii_locked_file(const char *fname, int fd, bool temp) : raii_file(fname, fd, temp) {}
 };
 
 /**
  * A mmap wrapper on top of a locked file
  */
-struct raii_mmaped_locked_file final {
-       ~raii_mmaped_locked_file();
-       static auto mmap_shared(raii_locked_file &&file, int flags) -> tl::expected<raii_mmaped_locked_file, std::string>;
-       static auto mmap_shared(const char *fname, int open_flags, int mmap_flags) -> tl::expected<raii_mmaped_locked_file, std::string>;
+struct raii_mmaped_file final {
+       ~raii_mmaped_file();
+       static auto mmap_shared(raii_file &&file, int flags) -> tl::expected<raii_mmaped_file, std::string>;
+       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;}
        // Passes the ownership of the mmaped memory to the callee
@@ -123,23 +178,23 @@ struct raii_mmaped_locked_file final {
 
        auto get_size() const -> std::size_t { return file.get_stat().st_size; }
 
-       raii_mmaped_locked_file& operator=(raii_mmaped_locked_file &&other) noexcept {
+       raii_mmaped_file& operator=(raii_mmaped_file &&other) noexcept {
                std::swap(map, other.map);
                file = std::move(other.file);
 
                return *this;
        }
 
-       raii_mmaped_locked_file(raii_mmaped_locked_file &&other) noexcept;
+       raii_mmaped_file(raii_mmaped_file &&other) noexcept;
 
        /* Do not allow copy/default ctor */
-       const raii_mmaped_locked_file& operator=(const raii_mmaped_locked_file &other) = delete;
-       raii_mmaped_locked_file() = delete;
-       raii_mmaped_locked_file(const raii_mmaped_locked_file &other) = delete;
+       const raii_mmaped_file& operator=(const raii_mmaped_file &other) = delete;
+       raii_mmaped_file() = delete;
+       raii_mmaped_file(const raii_mmaped_file &other) = delete;
 private:
        /* Is intended to be used with map_shared */
-       explicit raii_mmaped_locked_file(raii_locked_file &&_file, void *_map);
-       raii_locked_file file;
+       explicit raii_mmaped_file(raii_file &&_file, void *_map);
+       raii_file file;
        void *map = nullptr;
 };