From: Alexander Lanin Date: Tue, 19 Jan 2021 18:18:52 +0000 (+0100) Subject: refactor: Drop out parameter from get_level_1_files (#779) X-Git-Tag: v4.2~20 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=e4b02c3962df7f129682db5f48926b350368cc71;p=thirdparty%2Fccache.git refactor: Drop out parameter from get_level_1_files (#779) --- diff --git a/src/CacheFile.hpp b/src/CacheFile.hpp index 59c180bea..05410685a 100644 --- a/src/CacheFile.hpp +++ b/src/CacheFile.hpp @@ -21,7 +21,6 @@ #include "system.hpp" #include "Stat.hpp" -#include "exceptions.hpp" #include "third_party/nonstd/optional.hpp" @@ -34,15 +33,12 @@ public: explicit CacheFile(const std::string& path); - CacheFile(const CacheFile&) = delete; - CacheFile& operator=(const CacheFile&) = delete; - const Stat& lstat() const; const std::string& path() const; Type type() const; private: - const std::string m_path; + std::string m_path; mutable nonstd::optional m_stat; }; diff --git a/src/Util.cpp b/src/Util.cpp index 4be232edc..71de5f9cd 100644 --- a/src/Util.cpp +++ b/src/Util.cpp @@ -654,13 +654,14 @@ get_extension(string_view path) } } -void +std::vector get_level_1_files(const std::string& dir, - const ProgressReceiver& progress_receiver, - std::vector>& files) + const ProgressReceiver& progress_receiver) { + std::vector files; + if (!Stat::stat(dir)) { - return; + return files; } size_t level_2_directories = 0; @@ -672,7 +673,7 @@ get_level_1_files(const std::string& dir, } if (!is_dir) { - files.push_back(std::make_shared(path)); + files.emplace_back(path); } else if (path != dir && path.find('/', dir.size() + 1) == std::string::npos) { ++level_2_directories; @@ -681,6 +682,7 @@ get_level_1_files(const std::string& dir, }); progress_receiver(1.0); + return files; } std::string diff --git a/src/Util.hpp b/src/Util.hpp index 819950b5f..7db8d95af 100644 --- a/src/Util.hpp +++ b/src/Util.hpp @@ -208,10 +208,9 @@ nonstd::string_view get_extension(nonstd::string_view path); // Parameters: // - dir: The directory to traverse recursively. // - progress_receiver: Function that will be called for progress updates. -// - files: Found files. -void get_level_1_files(const std::string& dir, - const ProgressReceiver& progress_receiver, - std::vector>& files); +std::vector +get_level_1_files(const std::string& dir, + const ProgressReceiver& progress_receiver); // Return the current user's home directory, or throw `Fatal` if it can't // be determined. diff --git a/src/cleanup.cpp b/src/cleanup.cpp index a3ee774ee..5c76ebbeb 100644 --- a/src/cleanup.cpp +++ b/src/cleanup.cpp @@ -91,9 +91,8 @@ clean_up_dir(const std::string& subdir, { LOG("Cleaning up cache directory {}", subdir); - std::vector> files; - Util::get_level_1_files( - subdir, [&](double progress) { progress_receiver(progress / 3); }, files); + std::vector files = Util::get_level_1_files( + subdir, [&](double progress) { progress_receiver(progress / 3); }); uint64_t cache_size = 0; uint64_t files_in_cache = 0; @@ -103,29 +102,27 @@ clean_up_dir(const std::string& subdir, ++i, progress_receiver(1.0 / 3 + 1.0 * i / files.size() / 3)) { const auto& file = files[i]; - if (!file->lstat().is_regular()) { + if (!file.lstat().is_regular()) { // Not a file or missing file. continue; } // Delete any tmp files older than 1 hour right away. - if (file->lstat().mtime() + 3600 < current_time - && Util::base_name(file->path()).find(".tmp.") != std::string::npos) { - Util::unlink_tmp(file->path()); + if (file.lstat().mtime() + 3600 < current_time + && Util::base_name(file.path()).find(".tmp.") != std::string::npos) { + Util::unlink_tmp(file.path()); continue; } - cache_size += file->lstat().size_on_disk(); + cache_size += file.lstat().size_on_disk(); files_in_cache += 1; } // Sort according to modification time, oldest first. - std::sort(files.begin(), - files.end(), - [](const std::shared_ptr& f1, - const std::shared_ptr& f2) { - return f1->lstat().mtime() < f2->lstat().mtime(); - }); + std::sort( + files.begin(), files.end(), [](const CacheFile& f1, const CacheFile& f2) { + return f1.lstat().mtime() < f2.lstat().mtime(); + }); LOG("Before cleanup: {:.0f} KiB, {:.0f} files", static_cast(cache_size) / 1024, @@ -136,27 +133,26 @@ clean_up_dir(const std::string& subdir, ++i, progress_receiver(2.0 / 3 + 1.0 * i / files.size() / 3)) { const auto& file = files[i]; - if (!file->lstat() || file->lstat().is_directory()) { + if (!file.lstat() || file.lstat().is_directory()) { continue; } if ((max_size == 0 || cache_size <= max_size) && (max_files == 0 || files_in_cache <= max_files) && (max_age == 0 - || file->lstat().mtime() + || file.lstat().mtime() > (current_time - static_cast(max_age)))) { break; } - if (Util::ends_with(file->path(), ".stderr")) { + if (Util::ends_with(file.path(), ".stderr")) { // In order to be nice to legacy ccache versions, make sure that the .o // file is deleted before .stderr, because if the ccache process gets // killed after deleting the .stderr but before deleting the .o, the // cached result will be inconsistent. (.stderr is the only file that is // optional for legacy ccache versions; any other file missing from the // cache will be detected.) - std::string o_file = - file->path().substr(0, file->path().size() - 6) + "o"; + std::string o_file = file.path().substr(0, file.path().size() - 6) + "o"; // Don't subtract this extra deletion from the cache size; that // bookkeeping will be done when the loop reaches the .o file. If the @@ -168,7 +164,7 @@ clean_up_dir(const std::string& subdir, } delete_file( - file->path(), file->lstat().size_on_disk(), &cache_size, &files_in_cache); + file.path(), file.lstat().size_on_disk(), &cache_size, &files_in_cache); cleaned = true; } @@ -208,12 +204,11 @@ wipe_dir(const std::string& subdir, { LOG("Clearing out cache directory {}", subdir); - std::vector> files; - Util::get_level_1_files( - subdir, [&](double progress) { progress_receiver(progress / 2); }, files); + const std::vector files = Util::get_level_1_files( + subdir, [&](double progress) { progress_receiver(progress / 2); }); for (size_t i = 0; i < files.size(); ++i) { - Util::unlink_safe(files[i]->path()); + Util::unlink_safe(files[i].path()); progress_receiver(0.5 + 0.5 * i / files.size()); } diff --git a/src/compress.cpp b/src/compress.cpp index 42e0179cd..1164b7950 100644 --- a/src/compress.cpp +++ b/src/compress.cpp @@ -221,23 +221,20 @@ compress_stats(const Config& config, config.cache_dir(), [&](const std::string& subdir, const Util::ProgressReceiver& sub_progress_receiver) { - std::vector> files; - Util::get_level_1_files( - subdir, - [&](double progress) { sub_progress_receiver(progress / 2); }, - files); + const std::vector files = Util::get_level_1_files( + subdir, [&](double progress) { sub_progress_receiver(progress / 2); }); for (size_t i = 0; i < files.size(); ++i) { const auto& cache_file = files[i]; - on_disk_size += cache_file->lstat().size_on_disk(); + on_disk_size += cache_file.lstat().size_on_disk(); try { - auto file = open_file(cache_file->path(), "rb"); - auto reader = create_reader(*cache_file, file.get()); - compr_size += cache_file->lstat().size(); + auto file = open_file(cache_file.path(), "rb"); + auto reader = create_reader(cache_file, file.get()); + compr_size += cache_file.lstat().size(); content_size += reader->content_size(); } catch (Error&) { - incompr_size += cache_file->lstat().size(); + incompr_size += cache_file.lstat().size(); } sub_progress_receiver(1.0 / 2 + 1.0 * i / files.size() / 2); @@ -290,27 +287,26 @@ compress_recompress(Context& ctx, ctx.config.cache_dir(), [&](const std::string& subdir, const Util::ProgressReceiver& sub_progress_receiver) { - std::vector> files; - Util::get_level_1_files( - subdir, - [&](double progress) { sub_progress_receiver(0.1 * progress); }, - files); + std::vector files = + Util::get_level_1_files(subdir, [&](double progress) { + sub_progress_receiver(0.1 * progress); + }); auto stats_file = subdir + "/stats"; for (size_t i = 0; i < files.size(); ++i) { const auto& file = files[i]; - if (file->type() != CacheFile::Type::unknown) { + if (file.type() != CacheFile::Type::unknown) { thread_pool.enqueue([&statistics, stats_file, file, level] { try { - recompress_file(statistics, stats_file, *file, level); + recompress_file(statistics, stats_file, file, level); } catch (Error&) { // Ignore for now. } }); } else { - statistics.update(0, 0, 0, file->lstat().size()); + statistics.update(0, 0, 0, file.lstat().size()); } sub_progress_receiver(0.1 + 0.9 * i / files.size()); diff --git a/unittest/test_Util.cpp b/unittest/test_Util.cpp index ce3c1115d..5fd52399d 100644 --- a/unittest/test_Util.cpp +++ b/unittest/test_Util.cpp @@ -376,43 +376,40 @@ TEST_CASE("Util::get_level_1_files") Util::write_file("0/1/file_c", "12"); Util::write_file("0/f/c/file_d", "123"); - std::vector> files; auto null_receiver = [](double) {}; SUBCASE("nonexistent subdirectory") { - Util::get_level_1_files("2", null_receiver, files); + const auto files = Util::get_level_1_files("2", null_receiver); CHECK(files.empty()); } SUBCASE("empty subdirectory") { - Util::get_level_1_files("e", null_receiver, files); + const auto files = Util::get_level_1_files("e", null_receiver); CHECK(files.empty()); } SUBCASE("simple case") { - Util::get_level_1_files("0", null_receiver, files); + auto files = Util::get_level_1_files("0", null_receiver); REQUIRE(files.size() == 4); // Files within a level are in arbitrary order, sort them to be able to // verify them. - std::sort(files.begin(), - files.end(), - [](const std::shared_ptr& f1, - const std::shared_ptr& f2) { - return f1->path() < f2->path(); - }); - - CHECK(files[0]->path() == os_path("0/1/file_b")); - CHECK(files[0]->lstat().size() == 1); - CHECK(files[1]->path() == os_path("0/1/file_c")); - CHECK(files[1]->lstat().size() == 2); - CHECK(files[2]->path() == os_path("0/f/c/file_d")); - CHECK(files[2]->lstat().size() == 3); - CHECK(files[3]->path() == os_path("0/file_a")); - CHECK(files[3]->lstat().size() == 0); + std::sort( + files.begin(), files.end(), [](const CacheFile& f1, const CacheFile& f2) { + return f1.path() < f2.path(); + }); + + CHECK(files[0].path() == os_path("0/1/file_b")); + CHECK(files[0].lstat().size() == 1); + CHECK(files[1].path() == os_path("0/1/file_c")); + CHECK(files[1].lstat().size() == 2); + CHECK(files[2].path() == os_path("0/f/c/file_d")); + CHECK(files[2].lstat().size() == 3); + CHECK(files[3].path() == os_path("0/file_a")); + CHECK(files[3].lstat().size() == 0); } }