From ab48b79bd5fdacfb9ba4b17745656cb06f15df67 Mon Sep 17 00:00:00 2001
From: Joel Rosdahl <joel@rosdahl.net>
Date: Tue, 18 Jul 2023 10:19:05 +0200
Subject: [PATCH] refactor: Let util::rename return std::error_code

---
 src/AtomicFile.cpp                 |  6 ++++--
 src/storage/local/LocalStorage.cpp |  6 ++++--
 src/util/file.cpp                  | 19 +++++++++++++------
 src/util/file.hpp                  |  4 ++--
 4 files changed, 23 insertions(+), 12 deletions(-)

diff --git a/src/AtomicFile.cpp b/src/AtomicFile.cpp
index ef6fe8e12..73c5682bb 100644
--- a/src/AtomicFile.cpp
+++ b/src/AtomicFile.cpp
@@ -82,7 +82,9 @@ AtomicFile::commit()
   }
   const auto result = util::rename(m_tmp_path, m_path);
   if (!result) {
-    throw core::Error(
-      FMT("failed to rename {} to {}: {}", m_tmp_path, m_path, result.error()));
+    throw core::Error(FMT("failed to rename {} to {}: {}",
+                          m_tmp_path,
+                          m_path,
+                          result.error().message()));
   }
 }
diff --git a/src/storage/local/LocalStorage.cpp b/src/storage/local/LocalStorage.cpp
index 4513444d5..f6f7a7558 100644
--- a/src/storage/local/LocalStorage.cpp
+++ b/src/storage/local/LocalStorage.cpp
@@ -270,8 +270,10 @@ clone_file(const std::string& src, const std::string& dest, bool via_tmp_file)
   if (via_tmp_file) {
     const auto result = util::rename(tmp_file, dest);
     if (!result) {
-      throw core::Error(
-        FMT("failed to rename {} to {}: {}", tmp_file, dest, result.error()));
+      throw core::Error(FMT("failed to rename {} to {}: {}",
+                            tmp_file,
+                            dest,
+                            result.error().message()));
     }
   }
 #  elif defined(__APPLE__)
diff --git a/src/util/file.cpp b/src/util/file.cpp
index b0a718c44..d7f67d0d6 100644
--- a/src/util/file.cpp
+++ b/src/util/file.cpp
@@ -53,6 +53,7 @@
 
 #include <cerrno>
 #include <cstring>
+#include <filesystem>
 #include <fstream>
 #include <locale>
 #include <type_traits>
@@ -97,8 +98,10 @@ copy_file(const std::string& src,
   if (via_tmp_file == ViaTmpFile::yes) {
     const auto result = util::rename(tmp_file, dest);
     if (!result) {
-      return nonstd::make_unexpected(
-        FMT("Failed to rename {} to {}: {}", tmp_file, dest, result.error()));
+      return nonstd::make_unexpected(FMT("Failed to rename {} to {}: {}",
+                                         tmp_file,
+                                         dest,
+                                         result.error().message()));
     }
   }
 
@@ -369,12 +372,14 @@ read_file_part(const std::string& path, size_t pos, size_t count);
 template nonstd::expected<std::vector<uint8_t>, std::string>
 read_file_part(const std::string& path, size_t pos, size_t count);
 
-nonstd::expected<void, std::string>
+nonstd::expected<void, std::error_code>
 rename(const std::string& oldpath, const std::string& newpath)
 {
 #ifndef _WIN32
-  if (::rename(oldpath.c_str(), newpath.c_str()) != 0) {
-    return nonstd::make_unexpected(strerror(errno));
+  std::error_code ec;
+  std::filesystem::rename(oldpath, newpath, ec);
+  if (ec) {
+    return nonstd::make_unexpected(ec);
   }
 #else
   // Windows' rename() won't overwrite an existing file, so need to use
@@ -382,7 +387,9 @@ rename(const std::string& oldpath, const std::string& newpath)
   if (!MoveFileExA(
         oldpath.c_str(), newpath.c_str(), MOVEFILE_REPLACE_EXISTING)) {
     DWORD error = GetLastError();
-    return nonstd::make_unexpected(Win32Util::error_message(error));
+    // TODO: How should the Win32 error be mapped to std::error_code?
+    return nonstd::make_unexpected(
+      std::error_code(error, std::system_category()));
   }
 #endif
   return {};
diff --git a/src/util/file.hpp b/src/util/file.hpp
index 858083c21..347f5d91d 100644
--- a/src/util/file.hpp
+++ b/src/util/file.hpp
@@ -89,8 +89,8 @@ read_file_part(const std::string& path, size_t pos, size_t count);
 // Note: Mingw-w64's std::filesystem::rename is buggy and doesn't properly
 // overwrite an existing file, at least in version 9.1.0, hence this utility
 // function.
-nonstd::expected<void, std::string> rename(const std::string& oldpath,
-                                           const std::string& newpath);
+nonstd::expected<void, std::error_code> rename(const std::string& oldpath,
+                                               const std::string& newpath);
 
 // Set the FD_CLOEXEC on file descriptor `fd`. This is a NOP on Windows.
 void set_cloexec_flag(int fd);
-- 
2.47.2