From: Joel Rosdahl Date: Fri, 4 Aug 2023 07:03:31 +0000 (+0200) Subject: refactor: fs::path-ify some util file functions X-Git-Tag: v4.9~71 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=230ae9cafaa8ba50724ff2951d5371cd663e9222;p=thirdparty%2Fccache.git refactor: fs::path-ify some util file functions --- diff --git a/src/AtomicFile.hpp b/src/AtomicFile.hpp index 033a3fa3c..89dd05902 100644 --- a/src/AtomicFile.hpp +++ b/src/AtomicFile.hpp @@ -1,4 +1,4 @@ -// Copyright (C) 2019-2022 Joel Rosdahl and other contributors +// Copyright (C) 2019-2023 Joel Rosdahl and other contributors // // See doc/AUTHORS.adoc for a complete list of contributors. // @@ -22,6 +22,7 @@ #include #include +#include #include // This class represents a file whose data will be atomically written to a path @@ -47,7 +48,7 @@ public: private: const std::string m_path; - std::string m_tmp_path; + std::filesystem::path m_tmp_path; FILE* m_stream; }; diff --git a/src/ccache.cpp b/src/ccache.cpp index 378a0867c..24511f995 100644 --- a/src/ccache.cpp +++ b/src/ccache.cpp @@ -708,7 +708,7 @@ result_key_from_depfile(Context& ctx, Hash& hash) struct GetTmpFdResult { Fd fd; - std::string path; + fs::path path; }; static GetTmpFdResult @@ -719,7 +719,7 @@ get_tmp_fd(Context& ctx, if (capture_output) { TemporaryFile tmp_stdout( FMT("{}/{}", ctx.config.temporary_dir(), description)); - ctx.register_pending_tmp_file(tmp_stdout.path); + ctx.register_pending_tmp_file(tmp_stdout.path.string()); return {std::move(tmp_stdout.fd), std::move(tmp_stdout.path)}; } else { const auto dev_null_path = util::get_dev_null_path(); @@ -1232,7 +1232,7 @@ get_result_key_from_cpp(Context& ctx, Args& args, Hash& hash) // its thing correctly. TemporaryFile tmp_stdout(FMT("{}/cpp_stdout", ctx.config.temporary_dir()), FMT(".{}", ctx.config.cpp_extension())); - preprocessed_path = tmp_stdout.path; + preprocessed_path = tmp_stdout.path.string(); tmp_stdout.fd.close(); // We're only using the path. ctx.register_pending_tmp_file(preprocessed_path); diff --git a/src/execute.cpp b/src/execute.cpp index 8e04eddce..78092a73c 100644 --- a/src/execute.cpp +++ b/src/execute.cpp @@ -217,7 +217,7 @@ win32execute(const char* path, std::string args = Win32Util::argv_to_string(argv, sh); std::string full_path = Win32Util::add_exe_suffix(path); - std::string tmp_file_path; + fs::path tmp_file_path; Finalizer tmp_file_remover([&tmp_file_path] { if (!tmp_file_path.empty()) { diff --git a/src/util/file.cpp b/src/util/file.cpp index 107181c59..ec4037889 100644 --- a/src/util/file.cpp +++ b/src/util/file.cpp @@ -68,27 +68,25 @@ namespace fs = util::filesystem; namespace util { tl::expected -copy_file(const std::string& src, - const std::string& dest, - ViaTmpFile via_tmp_file) +copy_file(const fs::path& src, const fs::path& dest, ViaTmpFile via_tmp_file) { - Fd src_fd(open(src.c_str(), O_RDONLY | O_BINARY)); + Fd src_fd(open(src.string().c_str(), O_RDONLY | O_BINARY)); if (!src_fd) { return tl::unexpected( FMT("Failed to open {} for reading: {}", src, strerror(errno))); } - unlink(dest.c_str()); + unlink(dest.string().c_str()); Fd dest_fd; - std::string tmp_file; + fs::path tmp_file; if (via_tmp_file == ViaTmpFile::yes) { TemporaryFile temp_file(dest); dest_fd = std::move(temp_file.fd); tmp_file = temp_file.path; } else { - dest_fd = - Fd(open(dest.c_str(), O_WRONLY | O_CREAT | O_TRUNC | O_BINARY, 0666)); + dest_fd = Fd(open( + dest.string().c_str(), O_WRONLY | O_CREAT | O_TRUNC | O_BINARY, 0666)); if (!dest_fd) { return tl::unexpected( FMT("Failed to open {} for writing: {}", dest, strerror(errno))); @@ -115,7 +113,7 @@ copy_file(const std::string& src, } void -create_cachedir_tag(const std::string& dir) +create_cachedir_tag(const fs::path& dir) { constexpr char cachedir_tag[] = "Signature: 8a477f597d28d172789f06886806bc55\n" @@ -123,8 +121,8 @@ create_cachedir_tag(const std::string& dir) "# For information about cache directory tags, see:\n" "#\thttp://www.brynosaurus.com/cachedir/\n"; - const std::string path = FMT("{}/CACHEDIR.TAG", dir); - if (DirEntry(path).exists()) { + auto path = dir / "CACHEDIR.TAG"; + if (fs::exists(path)) { return; } const auto result = write_file(path, cachedir_tag); @@ -229,7 +227,7 @@ has_utf16_le_bom(std::string_view text) template tl::expected -read_file(const std::string& path, size_t size_hint) +read_file(const fs::path& path, size_t size_hint) { if (size_hint == 0) { DirEntry de(path); @@ -249,7 +247,7 @@ read_file(const std::string& path, size_t size_hint) return O_RDONLY | O_BINARY; } }(); - Fd fd(open(path.c_str(), open_flags)); + Fd fd(open(path.string().c_str(), open_flags)); if (!fd) { return tl::unexpected(strerror(errno)); } @@ -324,25 +322,25 @@ read_file(const std::string& path, size_t size_hint) return result; } -template tl::expected read_file(const std::string& path, +template tl::expected read_file(const fs::path& path, size_t size_hint); -template tl::expected -read_file(const std::string& path, size_t size_hint); +template tl::expected read_file(const fs::path& path, + size_t size_hint); template tl::expected, std::string> -read_file(const std::string& path, size_t size_hint); +read_file(const fs::path& path, size_t size_hint); template tl::expected -read_file_part(const std::string& path, size_t pos, size_t count) +read_file_part(const fs::path& path, size_t pos, size_t count) { T result; if (count == 0) { return result; } - Fd fd(open(path.c_str(), O_RDONLY | O_BINARY)); + Fd fd(open(path.string().c_str(), O_RDONLY | O_BINARY)); if (!fd) { LOG("Failed to open {}: {}", path, strerror(errno)); return tl::unexpected(strerror(errno)); @@ -380,16 +378,16 @@ read_file_part(const std::string& path, size_t pos, size_t count) } template tl::expected -read_file_part(const std::string& path, size_t pos, size_t count); +read_file_part(const fs::path& path, size_t pos, size_t count); template tl::expected -read_file_part(const std::string& path, size_t pos, size_t count); +read_file_part(const fs::path& path, size_t pos, size_t count); template tl::expected, std::string> -read_file_part(const std::string& path, size_t pos, size_t count); +read_file_part(const fs::path& path, size_t pos, size_t count); tl::expected -remove(const std::string& path, LogFailure log_failure) +remove(const fs::path& path, LogFailure log_failure) { auto result = fs::remove(path); if (result || log_failure == LogFailure::yes) { @@ -402,32 +400,33 @@ remove(const std::string& path, LogFailure log_failure) } tl::expected -remove_nfs_safe(const std::string& path, LogFailure log_failure) +remove_nfs_safe(const fs::path& path, LogFailure log_failure) { // fs::remove isn't atomic if path is on an NFS share, so we rename to a // temporary file. We don't care if the temporary file is trashed, so it's // always safe to remove it first. - std::string tmp_name = - FMT("{}.ccache{}remove", path, TemporaryFile::tmp_file_infix); + auto tmp_path = + path.parent_path() + / FMT("{}.ccache{}remove", path.filename(), TemporaryFile::tmp_file_infix); - auto rename_result = fs::rename(path, tmp_name); + auto rename_result = fs::rename(path, tmp_path); if (!rename_result) { // It's OK if it was removed in a race. if (rename_result.error().value() != ENOENT && rename_result.error().value() != ESTALE && log_failure == LogFailure::yes) { - LOG("Removing {} via {}", path, tmp_name); + LOG("Removing {} via {}", path, tmp_path); LOG("Renaming {} to {} failed: {}", path, - tmp_name, + tmp_path, rename_result.error().message()); } return tl::unexpected(rename_result.error()); } - auto remove_result = fs::remove(tmp_name); + auto remove_result = fs::remove(tmp_path); if (remove_result || log_failure == LogFailure::yes) { - LOG("Removing {} via {}", path, tmp_name); + LOG("Removing {} via {}", path, tmp_path); if (!remove_result) { LOG("Removal failed: {}", remove_result.error().message()); } @@ -436,7 +435,7 @@ remove_nfs_safe(const std::string& path, LogFailure log_failure) } void -set_timestamps(const std::string& path, +set_timestamps(const fs::path& path, std::optional mtime, std::optional atime) { @@ -446,7 +445,7 @@ set_timestamps(const std::string& path, atime_mtime[0] = (atime ? *atime : *mtime).to_timespec(); atime_mtime[1] = mtime->to_timespec(); } - utimensat(AT_FDCWD, path.c_str(), mtime ? atime_mtime : nullptr, 0); + utimensat(AT_FDCWD, path.string().c_str(), mtime ? atime_mtime : nullptr, 0); #elif defined(HAVE_UTIMES) timeval atime_mtime[2]; if (mtime) { @@ -456,15 +455,15 @@ set_timestamps(const std::string& path, atime_mtime[1].tv_sec = mtime->sec(); atime_mtime[1].tv_usec = mtime->nsec_decimal_part() / 1000; } - utimes(path.c_str(), mtime ? atime_mtime : nullptr); + utimes(path.string().c_str(), mtime ? atime_mtime : nullptr); #else utimbuf atime_mtime; if (mtime) { atime_mtime.actime = atime ? atime->sec() : mtime->sec(); atime_mtime.modtime = mtime->sec(); - utime(path.c_str(), &atime_mtime); + utime(path.string().c_str(), &atime_mtime); } else { - utime(path.c_str(), nullptr); + utime(path.string().c_str(), nullptr); } #endif } @@ -472,10 +471,10 @@ set_timestamps(const std::string& path, #ifdef HAVE_DIRENT_H tl::expected -traverse_directory(const std::string& directory, +traverse_directory(const fs::path& directory, const TraverseDirectoryVisitor& visitor) { - DIR* dir = opendir(directory.c_str()); + DIR* dir = opendir(directory.string().c_str()); if (!dir) { return tl::unexpected( FMT("Failed to traverse {}: {}", directory, strerror(errno))); @@ -490,7 +489,7 @@ traverse_directory(const std::string& directory, continue; } - std::string entry_path = directory + "/" + entry->d_name; + auto path = directory / entry->d_name; bool is_dir; # ifdef _DIRENT_HAVE_D_TYPE if (entry->d_type != DT_UNKNOWN) { @@ -498,25 +497,24 @@ traverse_directory(const std::string& directory, } else # endif { - DirEntry dir_entry(entry_path); + DirEntry dir_entry(path); if (!dir_entry) { if (dir_entry.error_number() == ENOENT || dir_entry.error_number() == ESTALE) { continue; } - return tl::unexpected(FMT("Failed to lstat {}: {}", - entry_path, - strerror(dir_entry.error_number()))); + return tl::unexpected(FMT( + "Failed to lstat {}: {}", path, strerror(dir_entry.error_number()))); } is_dir = dir_entry.is_directory(); } if (is_dir) { - traverse_directory(entry_path, visitor); + traverse_directory(path, visitor); } else { - visitor(entry_path, false); + visitor(path.string(), false); } } - visitor(directory, true); + visitor(directory.string(), true); return {}; } @@ -524,7 +522,7 @@ traverse_directory(const std::string& directory, #else // If not available, use the C++17 std::filesystem implementation. tl::expected -traverse_directory(const std::string& directory, +traverse_directory(const fs::path& directory, const TraverseDirectoryVisitor& visitor) { // Note: Intentionally not using std::filesystem::recursive_directory_iterator @@ -546,7 +544,7 @@ traverse_directory(const std::string& directory, visitor(entry.path().string(), entry.is_directory()); } } - visitor(directory, true); + visitor(directory.string(), true); } catch (const std::filesystem::filesystem_error& e) { return tl::unexpected(e.what()); } @@ -575,12 +573,13 @@ write_fd(int fd, const void* data, size_t size) } tl::expected -write_file(const std::string& path, std::string_view data, InPlace in_place) +write_file(const fs::path& path, std::string_view data, InPlace in_place) { if (in_place == InPlace::no) { - unlink(path.c_str()); + unlink(path.string().c_str()); } - Fd fd(open(path.c_str(), O_WRONLY | O_CREAT | O_TRUNC | O_TEXT, 0666)); + Fd fd( + open(path.string().c_str(), O_WRONLY | O_CREAT | O_TRUNC | O_TEXT, 0666)); if (!fd) { return tl::unexpected(strerror(errno)); } @@ -588,14 +587,15 @@ write_file(const std::string& path, std::string_view data, InPlace in_place) } tl::expected -write_file(const std::string& path, +write_file(const fs::path& path, nonstd::span data, InPlace in_place) { if (in_place == InPlace::no) { - unlink(path.c_str()); + unlink(path.string().c_str()); } - Fd fd(open(path.c_str(), O_WRONLY | O_CREAT | O_TRUNC | O_BINARY, 0666)); + Fd fd( + open(path.string().c_str(), O_WRONLY | O_CREAT | O_TRUNC | O_BINARY, 0666)); if (!fd) { return tl::unexpected(strerror(errno)); } diff --git a/src/util/file.hpp b/src/util/file.hpp index 50e3afb29..7fd25003b 100644 --- a/src/util/file.hpp +++ b/src/util/file.hpp @@ -28,6 +28,7 @@ #include #include #include +#include #include #include #include @@ -48,11 +49,11 @@ using TraverseDirectoryVisitor = // Copy a file from `src` to `dest`. If `via_tmp_file` is yes, `src` is copied // to a temporary file and then renamed to dest. tl::expected -copy_file(const std::string& src, - const std::string& dest, +copy_file(const std::filesystem::path& src, + const std::filesystem::path& dest, ViaTmpFile via_tmp_file = ViaTmpFile::no); -void create_cachedir_tag(const std::string& dir); +void create_cachedir_tag(const std::filesystem::path& 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. @@ -83,7 +84,7 @@ tl::expected read_fd(int fd); // If `size_hint` is not 0 then it is assumed that `path` has this size (this // saves system calls). template -tl::expected read_file(const std::string& path, +tl::expected read_file(const std::filesystem::path& path, size_t size_hint = 0); // Return (at most) `count` bytes from `path` starting at position `pos`. @@ -94,7 +95,7 @@ tl::expected read_file(const std::string& path, // UTF-8. template tl::expected -read_file_part(const std::string& path, size_t pos, size_t count); +read_file_part(const std::filesystem::path& path, size_t pos, size_t count); // Remove `path` (non-directory), NFS hazardous. Use only for files that will // not exist on other systems. @@ -102,14 +103,15 @@ read_file_part(const std::string& path, size_t pos, size_t count); // Returns whether the file was removed. A nonexistent `path` is considered // successful. tl::expected -remove(const std::string& path, LogFailure log_failure = LogFailure::yes); +remove(const std::filesystem::path& path, + LogFailure log_failure = LogFailure::yes); // Remove `path` (non-directory), NFS safe. // // Returns whether the file was removed. A nonexistent `path` is considered a // successful. tl::expected -remove_nfs_safe(const std::string& path, +remove_nfs_safe(const std::filesystem::path& path, LogFailure log_failure = LogFailure::yes); // Set the FD_CLOEXEC on file descriptor `fd`. This is a NOP on Windows. @@ -117,14 +119,14 @@ void set_cloexec_flag(int fd); // Set atime/mtime of `path`. If `mtime` is std::nullopt, set to the current // time. If `atime` is std::nullopt, set to what `mtime` specifies. -void set_timestamps(const std::string& path, +void set_timestamps(const std::filesystem::path& path, std::optional mtime = std::nullopt, std::optional atime = std::nullopt); // Traverse `path` recursively in postorder (directory entries are visited // before their parent directory). tl::expected -traverse_directory(const std::string& directory, +traverse_directory(const std::filesystem::path& directory, const TraverseDirectoryVisitor& visitor); // Write `size` bytes from binary `data` to `fd`. @@ -132,13 +134,13 @@ tl::expected 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 write_file(const std::string& path, +tl::expected 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 write_file(const std::string& path, +tl::expected write_file(const std::filesystem::path& path, nonstd::span data, InPlace in_place = InPlace::no); diff --git a/unittest/test_util_file.cpp b/unittest/test_util_file.cpp index 798ac88f1..ed78dff2e 100644 --- a/unittest/test_util_file.cpp +++ b/unittest/test_util_file.cpp @@ -40,20 +40,6 @@ namespace fs = util::filesystem; using TestUtil::TestContext; using util::DirEntry; -namespace { - -std::string -os_path(std::string path) -{ -#if defined(_WIN32) && !defined(HAVE_DIRENT_H) - std::replace(path.begin(), path.end(), '/', '\\'); -#endif - - return path; -} - -} // namespace - TEST_CASE("util::fallocate") { TestContext test_context; @@ -260,8 +246,8 @@ TEST_CASE("util::traverse_directory") { CHECK_NOTHROW(util::traverse_directory("dir-with-files", visitor)); REQUIRE(visited.size() == 3); - std::string f1 = os_path("[f] dir-with-files/f1"); - std::string f2 = os_path("[f] dir-with-files/f2"); + fs::path f1("[f] dir-with-files/f1"); + fs::path f2("[f] dir-with-files/f2"); CHECK(((visited[0] == f1 && visited[1] == f2) || (visited[0] == f2 && visited[1] == f1))); CHECK(visited[2] == "[d] dir-with-files"); @@ -272,8 +258,8 @@ TEST_CASE("util::traverse_directory") CHECK_NOTHROW( util::traverse_directory("dir-with-subdir-and-file", visitor)); REQUIRE(visited.size() == 3); - CHECK(visited[0] == os_path("[f] dir-with-subdir-and-file/subdir/f")); - CHECK(visited[1] == os_path("[d] dir-with-subdir-and-file/subdir")); + CHECK(visited[0] == fs::path("[f] dir-with-subdir-and-file/subdir/f")); + CHECK(visited[1] == fs::path("[d] dir-with-subdir-and-file/subdir")); CHECK(visited[2] == "[d] dir-with-subdir-and-file"); } }