refactor: Move Util::fallocate to util

diff --git a/src/InodeCache.cpp b/src/InodeCache.cpp
index 44b713069ea7c800fcbe263996dfbb0e63ab49b1..d0c0856cb00803bf028e11105dc798ad5b3066c2 100644 (file)
--- a/src/InodeCache.cpp
+++ b/src/InodeCache.cpp
@@ -28,6 +28,7 @@
 #include "fmtmacros.hpp"
 
 #include <util/conversion.hpp>
+#include <util/file.hpp>
 
 #include <fcntl.h>
 #include <libgen.h>
@@ -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 75afc0003590bb7f814858c618deea06de686b60..9b157b6dbbb714d7f53584c389547113b48358c7 100644 (file)
--- 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 14008c928215cfc650bd11a976386ae2b2384be8..ae8a9d58fdc2f227e6c65c44e5f5eb3270c5bf23 100644 (file)
--- 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 ed6d8f93281a8fa6672a0eacca30344afeff3c37..49c11942d45ac9fd3dbaf3dac296386c40b458f9 100644 (file)
--- a/src/util/file.cpp
+++ b/src/util/file.cpp
@@ -19,6 +19,7 @@
 #include "file.hpp"
 
 #include <Fd.hpp>
+#include <Finalizer.hpp>
 #include <Logging.hpp>
 #include <Stat.hpp>
 #include <TemporaryFile.hpp>
@@ -124,6 +125,47 @@ create_cachedir_tag(const std::string& dir)
   }
 }
 
+nonstd::expected<void, std::string>
+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<size_t>(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<void, std::string>
 read_fd(int fd, DataReceiver data_receiver)
 {

diff --git a/src/util/file.hpp b/src/util/file.hpp
index fd81cfaa1d496d97b911979e94730cc85b5c13ae..684eb71f2820368ffd84371ccad964150cd42151 100644 (file)
--- 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<void, std::string> 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 01f89076a0abc0f42e6a7205f2ed99d9cb11484d..a8b215d5865f298ea3978ee30f0290338bbf6fa6 100644 (file)
--- 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 8b86835354f775ccc2d8bf8f11d07842e9c93571..d864681d8fcb4acb8ccc1597b2126e5e35d7e6ef 100644 (file)
--- a/unittest/test_util_file.cpp
+++ b/unittest/test_util_file.cpp
@@ -18,12 +18,16 @@
 
 #include "TestUtil.hpp"
 
+#include <Fd.hpp>
+#include <Stat.hpp>
 #include <util/Bytes.hpp>
 #include <util/file.hpp>
 #include <util/string.hpp>
 
 #include <third_party/doctest.h>
 
+#include <fcntl.h>
+
 #include <cstring>
 #include <string>
 #include <string_view>
@@ -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);