]> git.ipfire.org Git - thirdparty/ccache.git/commitdiff
enhance: Implement O_EXCL mode for util::write_file
authorJoel Rosdahl <joel@rosdahl.net>
Sat, 23 Mar 2024 15:29:59 +0000 (16:29 +0100)
committerJoel Rosdahl <joel@rosdahl.net>
Sun, 24 Mar 2024 08:30:30 +0000 (09:30 +0100)
src/ccache/util/file.cpp
src/ccache/util/file.hpp
unittest/test_util_DirEntry.cpp
unittest/test_util_file.cpp

index 2b00e82f55a8d9dca678e234ddf2ed2f2b8cdc46..af5335a9e5ee0cfa278d7a671b9a646d66b99049 100644 (file)
@@ -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));
   }
index d72f40995be5df9aa38c9a9e2609a0ef04734a80..4707f4f5857a0f8501f0125d570cf9977c42669b 100644 (file)
@@ -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 ---
 
index 9418a2f75e4e44ad4cd88496259c1452c407ca53..6e7da3377002a23808831c0b23cf70a173578f10 100644 (file)
@@ -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);
 }
index 1a28997b5ab8ed6375319a12342137c118af69aa..8a9b9597b27d890c96a21d2b0561b330ea42150f 100644 (file)
 #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;