From: Joel Rosdahl <joel@rosdahl.net>
Date: Sat, 23 Mar 2024 15:29:59 +0000 (+0100)
Subject: enhance: Implement O_EXCL mode for util::write_file
X-Git-Tag: v4.10~67
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=8b95476767c7e4dea6df5317b1b1151c118d6dd8;p=thirdparty%2Fccache.git

enhance: Implement O_EXCL mode for util::write_file
---

diff --git a/src/ccache/util/file.cpp b/src/ccache/util/file.cpp
index 2b00e82f..af5335a9 100644
--- a/src/ccache/util/file.cpp
+++ b/src/ccache/util/file.cpp
@@ -585,13 +585,17 @@ write_fd(int fd, const void* data, size_t size)
 }
 
 tl::expected<void, std::string>
-write_file(const fs::path& path, std::string_view data, InPlace in_place)
+write_file(const fs::path& path, std::string_view data, WriteFileMode mode)
 {
   auto path_str = pstr(path);
-  if (in_place == InPlace::no) {
+  if (mode == WriteFileMode::unlink) {
     unlink(path_str);
   }
-  Fd fd(open(path_str, O_WRONLY | O_CREAT | O_TRUNC | O_TEXT, 0666));
+  int flags = O_WRONLY | O_CREAT | O_TRUNC | O_TEXT;
+  if (mode == WriteFileMode::exclusive) {
+    flags |= O_EXCL;
+  }
+  Fd fd(open(path_str, flags, 0666));
   if (!fd) {
     return tl::unexpected(strerror(errno));
   }
@@ -601,13 +605,17 @@ write_file(const fs::path& path, std::string_view data, InPlace in_place)
 tl::expected<void, std::string>
 write_file(const fs::path& path,
            nonstd::span<const uint8_t> data,
-           InPlace in_place)
+           WriteFileMode mode)
 {
   auto path_str = pstr(path);
-  if (in_place == InPlace::no) {
+  if (mode == WriteFileMode::unlink) {
     unlink(path_str);
   }
-  Fd fd(open(path_str, O_WRONLY | O_CREAT | O_TRUNC | O_BINARY, 0666));
+  int flags = O_WRONLY | O_CREAT | O_TRUNC | O_BINARY;
+  if (mode == WriteFileMode::exclusive) {
+    flags |= O_EXCL;
+  }
+  Fd fd(open(path_str, flags, 0666));
   if (!fd) {
     return tl::unexpected(strerror(errno));
   }
diff --git a/src/ccache/util/file.hpp b/src/ccache/util/file.hpp
index d72f4099..4707f4f5 100644
--- a/src/ccache/util/file.hpp
+++ b/src/ccache/util/file.hpp
@@ -39,7 +39,11 @@ namespace util {
 
 // --- Interface ---
 
-enum class InPlace { yes, no };
+enum class WriteFileMode {
+  unlink,    // unlink existing file before writing (break hard links)
+  in_place,  // don't unlink before writing (don't break hard links)
+  exclusive, // return error if the file already exists (O_EXCL)
+};
 enum class LogFailure { yes, no };
 enum class ViaTmpFile { yes, no };
 
@@ -131,17 +135,17 @@ traverse_directory(const std::filesystem::path& directory,
 // Write `size` bytes from binary `data` to `fd`.
 tl::expected<void, std::string> write_fd(int fd, const void* data, size_t size);
 
-// Write text `data` to `path`. If `in_place` is no, unlink any existing file
-// first (i.e., break hard links).
-tl::expected<void, std::string> write_file(const std::filesystem::path& path,
-                                           std::string_view data,
-                                           InPlace in_place = InPlace::no);
-
-// Write binary `data` to `path`. If `in_place` is no, unlink any existing
-// file first (i.e., break hard links).
-tl::expected<void, std::string> write_file(const std::filesystem::path& path,
-                                           nonstd::span<const uint8_t> data,
-                                           InPlace in_place = InPlace::no);
+// Write text `data` to `path`.
+tl::expected<void, std::string>
+write_file(const std::filesystem::path& path,
+           std::string_view data,
+           WriteFileMode mode = WriteFileMode::unlink);
+
+// Write binary `data` to `path`.
+tl::expected<void, std::string>
+write_file(const std::filesystem::path& path,
+           nonstd::span<const uint8_t> data,
+           WriteFileMode mode = WriteFileMode::unlink);
 
 // --- Inline implementations ---
 
diff --git a/unittest/test_util_DirEntry.cpp b/unittest/test_util_DirEntry.cpp
index 9418a2f7..6e7da337 100644
--- a/unittest/test_util_DirEntry.cpp
+++ b/unittest/test_util_DirEntry.cpp
@@ -229,7 +229,7 @@ TEST_CASE("Caching and refresh")
   DirEntry entry("a");
   CHECK(entry.size() == 0);
 
-  util::write_file("a", "123", util::InPlace::yes);
+  util::write_file("a", "123", util::WriteFileMode::in_place);
   CHECK(entry.size() == 0);
   entry.refresh();
   CHECK(entry.size() == 3);
@@ -247,7 +247,7 @@ TEST_CASE("Same i-node as")
   CHECK(entry_a.same_inode_as(entry_a));
   CHECK(!entry_a.same_inode_as(entry_b));
 
-  util::write_file("a", "change size", util::InPlace::yes);
+  util::write_file("a", "change size", util::WriteFileMode::in_place);
   CHECK(DirEntry("a").same_inode_as(entry_a));
 
   CHECK(!DirEntry("nonexistent").same_inode_as(DirEntry("nonexistent")));
@@ -460,7 +460,7 @@ TEST_CASE("Hard links")
   CHECK(entry_a.inode() == entry_b.inode());
   CHECK(entry_a.same_inode_as(entry_b));
 
-  util::write_file("a", "1234567", util::InPlace::yes);
+  util::write_file("a", "1234567", util::WriteFileMode::in_place);
   entry_b.refresh();
   CHECK(entry_b.size() == 7);
 }
diff --git a/unittest/test_util_file.cpp b/unittest/test_util_file.cpp
index 1a28997b..8a9b9597 100644
--- a/unittest/test_util_file.cpp
+++ b/unittest/test_util_file.cpp
@@ -29,6 +29,10 @@
 #include <doctest.h>
 #include <fcntl.h>
 
+#ifndef _WIN32
+#  include <unistd.h>
+#endif
+
 #include <cstring>
 #include <string>
 #include <string_view>
@@ -204,6 +208,37 @@ TEST_CASE("util::read_file_part")
   }
 }
 
+#ifndef _WIN32
+TEST_CASE("util::write_file modes")
+{
+  TestContext test_context;
+
+  CHECK(util::write_file("test", "foo"));
+  CHECK(link("test", "test2") == 0);
+
+  SUBCASE("WriteFileMode::unlink")
+  {
+    CHECK(util::write_file("test", "bar", util::WriteFileMode::unlink));
+    CHECK(*util::read_file<std::string>("test2") == "foo");
+  }
+
+  SUBCASE("WriteFileMode::in_place")
+  {
+    CHECK(util::write_file("test", "bar", util::WriteFileMode::in_place));
+    CHECK(*util::read_file<std::string>("test2") == "bar");
+  }
+
+  SUBCASE("WriteFileMode::exclusive")
+  {
+    auto result =
+      util::write_file("test", "bar", util::WriteFileMode::exclusive);
+    CHECK(result.error() == "File exists");
+    CHECK(util::write_file("test3", "bar", util::WriteFileMode::exclusive));
+    CHECK(*util::read_file<std::string>("test3") == "bar");
+  }
+}
+#endif
+
 TEST_CASE("util::traverse_directory")
 {
   TestContext test_context;