Do not use file::path() in file::move (#175)

This is the next step to removing the path() method of the temporary
file (#165): now we can move a
temporary file without using its path

Technically, it always copies the file at this point, since I couldn't
figure out how to hardlink a file without hardlinks

Author: bugdea1er

src/file.cpp

@@ -33,24 +33,29 @@ constexpr std::size_t block_size = 4096;
 /// A type of buffer for I/O file operations
 using buffer_type = std::array<std::byte, block_size>;
 
-/// Opens a file in read-only mode
-/// @param[in]  path The path to the file to open
-/// @param[out] ec   Parameter for error reporting
+/// Opens a file and returns its handle
+/// @param[in]  path     The path to the file to open
+/// @param[in]  readonly Whether to open file only for reading
+/// @param[out] ec       Parameter for error reporting
 /// @returns A handle to the open file
-file::native_handle_type open(const fs::path& path,
+file::native_handle_type open(const fs::path& path, bool readonly,
                               std::error_code& ec) noexcept {
   ec.clear();
 
 #ifdef _WIN32
+  DWORD access = readonly ? GENERIC_READ : GENERIC_WRITE;
+  DWORD creation_disposition = readonly ? OPEN_EXISTING : CREATE_ALWAYS;
+  DWORD share_mode = FILE_SHARE_READ | FILE_SHARE_WRITE | FILE_SHARE_DELETE;
+
   HANDLE handle =
-      CreateFile(path.c_str(), GENERIC_READ,
-                 FILE_SHARE_READ | FILE_SHARE_WRITE | FILE_SHARE_DELETE, NULL,
-                 OPEN_EXISTING, FILE_ATTRIBUTE_NORMAL, NULL);
+      CreateFile(path.c_str(), access, share_mode, nullptr,
+                 creation_disposition, FILE_ATTRIBUTE_NORMAL, nullptr);
   if (handle == INVALID_HANDLE_VALUE) {
     ec = std::error_code(GetLastError(), std::system_category());
   }
 #else
-  int handle = ::open(path.c_str(), O_RDONLY | O_NONBLOCK);
+  int oflag = readonly ? O_RDONLY | O_NONBLOCK : O_RDWR | O_TRUNC | O_CREAT;
+  int handle = ::open(path.c_str(), oflag);
   if (handle == -1) {
     ec = std::error_code(errno, std::system_category());
   }
@@ -69,28 +74,23 @@ void close(file::native_handle_type handle) noexcept {
 #endif
 }
 
-/// Copies a file contents from the given path to the given file descriptor
-/// @param[in]  from The path to the source file
+/// Copies file contents from one file descriptor to another
+/// @param[in]  from The source file descriptor
 /// @param[in]  to   The target file descriptor
 /// @param[out] ec   Parameter for error reporting
-void copy_file(const fs::path& from, file::native_handle_type to,
+void copy_file(file::native_handle_type from, file::native_handle_type to,
                std::error_code& ec) noexcept {
   // TODO: can be optimized using `sendfile`, `copyfile` or other system API
-  file::native_handle_type source = open(from, ec);
-  if (ec) {
-    return;
-  }
-
   buffer_type buffer = buffer_type();
   while (true) {
 #ifdef _WIN32
     DWORD bytes_read;
-    if (!ReadFile(source, buffer.data(), block_size, &bytes_read, nullptr)) {
+    if (!ReadFile(from, buffer.data(), block_size, &bytes_read, nullptr)) {
       ec = std::error_code(GetLastError(), std::system_category());
       break;
     }
 #else
-    ssize_t bytes_read = read(source, buffer.data(), block_size);
+    ssize_t bytes_read = read(from, buffer.data(), block_size);
     if (bytes_read < 0) {
       ec = std::error_code(errno, std::system_category());
       break;
@@ -114,8 +114,32 @@ void copy_file(const fs::path& from, file::native_handle_type to,
     }
 #endif
   }
+}
 
-  close(source);
+/// Copies file contents from the given path to the given file descriptor
+/// @param[in]  from The path to the source file
+/// @param[in]  to   The target file descriptor
+/// @param[out] ec   Parameter for error reporting
+void copy_file(const fs::path& from, file::native_handle_type to,
+               std::error_code& ec) noexcept {
+  file::native_handle_type source = open(from, /*readonly=*/true, ec);
+  if (!ec) {
+    copy_file(source, to, ec);
+    close(source);
+  }
+}
+
+/// Copies file contents from the given path to the given file descriptor
+/// @param[in]  from The source file descriptor
+/// @param[in]  to   The path to the target file
+/// @param[out] ec   Parameter for error reporting
+void copy_file(file::native_handle_type from, const fs::path& to,
+               std::error_code& ec) noexcept {
+  file::native_handle_type target = open(to, /*readonly=*/false, ec);
+  if (!ec) {
+    copy_file(from, target, ec);
+    close(target);
+  }
 }
 }    // namespace
 
@@ -145,13 +169,16 @@ file::native_handle_type file::native_handle() const noexcept {
 }
 
 void file::move(const fs::path& to) {
-  sb.close();
+  // TODO: I couldn't figure out how to create a hard link to a file without
+  // hard links, so I just copy it even within the same file system
+
+  seekg(0, std::ios::beg);
 
   std::error_code ec;
-  tmp::move(*this, to, ec);
+  copy_file(native_handle(), to, ec);
 
   if (ec) {
-    throw fs::filesystem_error("Cannot move a temporary file", path(), to, ec);
+    throw fs::filesystem_error("Cannot move a temporary file", to, ec);
   }
 
   entry::clear();

tests/file.cpp

@@ -133,49 +133,20 @@ TEST(file, copy_errors) {
   EXPECT_THROW(file::copy("nonexistent.txt"), fs::filesystem_error);
 }
 
-/// Tests that moving a temporary file to itself does nothing
-TEST(file, move_to_self) {
-  fs::path path;
-
-  {
-    file tmpfile = file();
-    tmpfile << "Hello, world!";
-
-    path = tmpfile;
-
-    tmpfile.move(tmpfile);
-  }
-
-  EXPECT_TRUE(fs::exists(path));
-
-  {
-    std::ifstream stream = std::ifstream(path);
-    std::string content = std::string(std::istreambuf_iterator(stream), {});
-    EXPECT_EQ(content, "Hello, world!");
-  }
-
-  fs::remove_all(path);
-}
-
 /// Tests moving a temporary file to existing non-directory file
 TEST(file, move_to_existing_file) {
-  fs::path path;
-
   fs::path to = fs::path(BUILD_DIR) / "move_file_to_existing_test";
   std::ofstream(to) << "Goodbye, world!";
 
   {
     file tmpfile = file();
-    tmpfile << "Hello, world!";
-
-    path = tmpfile;
+    tmpfile << "Hello, world!" << std::flush;
 
     tmpfile.move(to);
   }
 
   std::error_code ec;
   EXPECT_TRUE(fs::exists(to, ec));
-  EXPECT_FALSE(fs::exists(path, ec));
 
   {
     std::ifstream stream = std::ifstream(to);