From b194ea3a696fdd29f32c71fd338177541c57788c Mon Sep 17 00:00:00 2001 From: Joel Rosdahl Date: Sat, 15 Jul 2023 10:41:25 +0200 Subject: [PATCH] refactor: Move Util::fallocate to util --- src/InodeCache.cpp | 8 ++++--- src/Util.cpp | 39 ---------------------------------- src/Util.hpp | 9 -------- src/util/file.cpp | 42 +++++++++++++++++++++++++++++++++++++ src/util/file.hpp | 7 +++++++ unittest/test_Util.cpp | 21 ------------------- unittest/test_util_file.cpp | 22 +++++++++++++++++++ 7 files changed, 76 insertions(+), 72 deletions(-) diff --git a/src/InodeCache.cpp b/src/InodeCache.cpp index 44b713069..d0c0856cb 100644 --- a/src/InodeCache.cpp +++ b/src/InodeCache.cpp @@ -28,6 +28,7 @@ #include "fmtmacros.hpp" #include +#include #include #include @@ -351,9 +352,10 @@ InodeCache::create_new_file(const std::string& filename) if (!fd_is_on_known_to_work_file_system(*tmp_file.fd)) { return false; } - int err = Util::fallocate(*tmp_file.fd, sizeof(SharedRegion)); - if (err != 0) { - LOG("Failed to allocate file space for inode cache: {}", strerror(err)); + + if (auto result = util::fallocate(*tmp_file.fd, sizeof(SharedRegion)); + !result) { + LOG("Failed to allocate file space for inode cache: {}", result.error()); return false; } SharedRegion* sr = diff --git a/src/Util.cpp b/src/Util.cpp index 75afc0003..9b157b6db 100644 --- a/src/Util.cpp +++ b/src/Util.cpp @@ -255,45 +255,6 @@ dir_name(std::string_view path) } } -int -fallocate(int fd, long new_size) -{ -#ifdef HAVE_POSIX_FALLOCATE - const int posix_fallocate_err = posix_fallocate(fd, 0, new_size); - if (posix_fallocate_err == 0 || posix_fallocate_err != EINVAL) { - return posix_fallocate_err; - } - // The underlying filesystem does not support the operation so fall back to - // lseek. -#endif - off_t saved_pos = lseek(fd, 0, SEEK_END); - off_t old_size = lseek(fd, 0, SEEK_END); - if (old_size == -1) { - int err = errno; - lseek(fd, saved_pos, SEEK_SET); - return err; - } - if (old_size >= new_size) { - lseek(fd, saved_pos, SEEK_SET); - return 0; - } - long bytes_to_write = new_size - old_size; - void* buf = calloc(bytes_to_write, 1); - if (!buf) { - lseek(fd, saved_pos, SEEK_SET); - return ENOMEM; - } - int err = 0; - try { - util::write_fd(fd, buf, bytes_to_write); - } catch (core::Error&) { - err = errno; - } - lseek(fd, saved_pos, SEEK_SET); - free(buf); - return err; -} - std::string format_argv_for_logging(const char* const* argv) { diff --git a/src/Util.hpp b/src/Util.hpp index 14008c928..ae8a9d58f 100644 --- a/src/Util.hpp +++ b/src/Util.hpp @@ -64,15 +64,6 @@ std::string_view dir_name(std::string_view path); // Like create_dir but throws Fatal on error. void ensure_dir_exists(std::string_view dir); -// Extends file size to at least new_size by calling posix_fallocate() if -// supported, otherwise by writing zeros last to the file. -// -// Note that existing holes are not filled in case posix_fallocate() is not -// supported. -// -// Returns 0 on success, an error number otherwise. -int fallocate(int fd, long new_size); - // Format `argv` as a simple string for logging purposes. That is, the result is // not intended to be machine parsable. `argv` must be terminated by a nullptr. std::string format_argv_for_logging(const char* const* argv); diff --git a/src/util/file.cpp b/src/util/file.cpp index ed6d8f932..49c11942d 100644 --- a/src/util/file.cpp +++ b/src/util/file.cpp @@ -19,6 +19,7 @@ #include "file.hpp" #include +#include #include #include #include @@ -124,6 +125,47 @@ create_cachedir_tag(const std::string& dir) } } +nonstd::expected +fallocate(int fd, size_t new_size) +{ +#ifdef HAVE_POSIX_FALLOCATE + const int posix_fallocate_err = posix_fallocate(fd, 0, new_size); + if (posix_fallocate_err == 0) { + return {}; + } + if (posix_fallocate_err != EINVAL) { + return nonstd::make_unexpected(strerror(posix_fallocate_err)); + } + // The underlying filesystem does not support the operation so fall back to + // lseek. +#endif + off_t saved_pos = lseek(fd, 0, SEEK_END); + off_t old_size = lseek(fd, 0, SEEK_END); + if (old_size == -1) { + int err = errno; + lseek(fd, saved_pos, SEEK_SET); + return nonstd::make_unexpected(strerror(err)); + } + if (static_cast(old_size) >= new_size) { + lseek(fd, saved_pos, SEEK_SET); + return {}; + } + long bytes_to_write = new_size - old_size; + + void* buf = calloc(bytes_to_write, 1); + if (!buf) { + lseek(fd, saved_pos, SEEK_SET); + return nonstd::make_unexpected(strerror(ENOMEM)); + } + Finalizer buf_freer([&] { free(buf); }); + + if (auto result = util::write_fd(fd, buf, bytes_to_write); !result) { + return result; + } + lseek(fd, saved_pos, SEEK_SET); + return {}; +} + nonstd::expected read_fd(int fd, DataReceiver data_receiver) { diff --git a/src/util/file.hpp b/src/util/file.hpp index fd81cfaa1..684eb71f2 100644 --- a/src/util/file.hpp +++ b/src/util/file.hpp @@ -47,6 +47,13 @@ copy_file(const std::string& src, void create_cachedir_tag(const std::string& dir); +// Extends file size of `fd` to at least `new_size` by calling posix_fallocate() +// if supported, otherwise by writing zeros last to the file. +// +// Note that existing holes are not filled in case posix_fallocate() is not +// supported. +nonstd::expected fallocate(int fd, size_t new_size); + // Return how much a file of `size` bytes likely would take on disk. uint64_t likely_size_on_disk(uint64_t size); diff --git a/unittest/test_Util.cpp b/unittest/test_Util.cpp index 01f89076a..a8b215d58 100644 --- a/unittest/test_Util.cpp +++ b/unittest/test_Util.cpp @@ -147,27 +147,6 @@ TEST_CASE("Util::ensure_dir_exists") "Failed to create directory create/dir/file: Not a directory"); } -TEST_CASE("Util::fallocate") -{ - TestContext test_context; - - const char filename[] = "test-file"; - - CHECK(Util::fallocate(Fd(creat(filename, S_IRUSR | S_IWUSR)).get(), 10000) - == 0); - CHECK(Stat::stat(filename).size() == 10000); - - CHECK( - Util::fallocate(Fd(open(filename, O_RDWR, S_IRUSR | S_IWUSR)).get(), 5000) - == 0); - CHECK(Stat::stat(filename).size() == 10000); - - CHECK( - Util::fallocate(Fd(open(filename, O_RDWR, S_IRUSR | S_IWUSR)).get(), 20000) - == 0); - CHECK(Stat::stat(filename).size() == 20000); -} - TEST_CASE("Util::format_argv_for_logging") { const char* argv_0[] = {nullptr}; diff --git a/unittest/test_util_file.cpp b/unittest/test_util_file.cpp index 8b8683535..d864681d8 100644 --- a/unittest/test_util_file.cpp +++ b/unittest/test_util_file.cpp @@ -18,12 +18,16 @@ #include "TestUtil.hpp" +#include +#include #include #include #include #include +#include + #include #include #include @@ -31,6 +35,24 @@ using TestUtil::TestContext; +TEST_CASE("util::fallocate") +{ + TestContext test_context; + + const char filename[] = "test-file"; + + CHECK(util::fallocate(Fd(creat(filename, S_IRUSR | S_IWUSR)).get(), 10000)); + CHECK(Stat::stat(filename).size() == 10000); + + CHECK( + util::fallocate(Fd(open(filename, O_RDWR, S_IRUSR | S_IWUSR)).get(), 5000)); + CHECK(Stat::stat(filename).size() == 10000); + + CHECK(util::fallocate(Fd(open(filename, O_RDWR, S_IRUSR | S_IWUSR)).get(), + 20000)); + CHECK(Stat::stat(filename).size() == 20000); +} + TEST_CASE("util::likely_size_on_disk") { CHECK(util::likely_size_on_disk(0) == 0); -- 2.47.2