From be678e8f9ce63223e85650710797a509a2bb1973 Mon Sep 17 00:00:00 2001 From: "Scott E. Graves" Date: Tue, 21 Jan 2025 08:01:15 -0600 Subject: [PATCH] refactor --- .../src/db/impl/sqlite_meta_db.cpp | 2 +- .../src/drives/directory_cache.cpp | 63 +++++++++++-------- .../src/drives/fuse/fuse_base.cpp | 2 +- .../src/drives/fuse/fuse_drive.cpp | 2 +- .../drives/remote/remote_open_file_table.cpp | 14 ++--- .../remotewinfsp/remote_winfsp_drive.cpp | 4 +- .../src/drives/winfsp/winfsp_drive.cpp | 2 +- repertory/librepertory/src/utils/polling.cpp | 2 +- .../src/fuse_drive_access_test.cpp | 4 +- .../src/fuse_drive_create_and_open_test.cpp | 18 +++--- .../src/winfsp_drive_create_test.cpp | 2 +- 11 files changed, 62 insertions(+), 53 deletions(-) diff --git a/repertory/librepertory/src/db/impl/sqlite_meta_db.cpp b/repertory/librepertory/src/db/impl/sqlite_meta_db.cpp index f0000bcf..6a66312d 100644 --- a/repertory/librepertory/src/db/impl/sqlite_meta_db.cpp +++ b/repertory/librepertory/src/db/impl/sqlite_meta_db.cpp @@ -343,7 +343,7 @@ auto sqlite_meta_db::set_item_meta(const std::string &api_path, // TODO handle error } - for (auto &&item : meta) { + for (const auto &item : meta) { existing_meta[item.first] = item.second; } diff --git a/repertory/librepertory/src/drives/directory_cache.cpp b/repertory/librepertory/src/drives/directory_cache.cpp index bdb1001c..e55430d9 100644 --- a/repertory/librepertory/src/drives/directory_cache.cpp +++ b/repertory/librepertory/src/drives/directory_cache.cpp @@ -38,14 +38,15 @@ void directory_cache::execute_action(const std::string &api_path, auto directory_cache::get_directory(std::uint64_t handle) -> std::shared_ptr { recur_mutex_lock directory_lock(directory_mutex_); - auto it = std::find_if(directory_lookup_.begin(), directory_lookup_.end(), - [handle](auto &&kv) -> bool { - auto &&handles = kv.second.handles; - return std::find(handles.begin(), handles.end(), - handle) != kv.second.handles.end(); - }); - if (it != directory_lookup_.end()) { - return it->second.iterator; + auto iter = + std::find_if(directory_lookup_.begin(), directory_lookup_.end(), + [handle](auto &&item) -> bool { + auto &&handles = item.second.handles; + return std::find(handles.begin(), handles.end(), handle) != + item.second.handles.end(); + }); + if (iter != directory_lookup_.end()) { + return iter->second.iterator; } return nullptr; @@ -66,17 +67,20 @@ auto directory_cache::remove_directory(const std::string &api_path) void directory_cache::remove_directory(std::uint64_t handle) { recur_mutex_lock directory_lock(directory_mutex_); - auto it = std::find_if(directory_lookup_.begin(), directory_lookup_.end(), - [handle](auto &&kv) -> bool { - auto &&handles = kv.second.handles; - return std::find(handles.begin(), handles.end(), - handle) != kv.second.handles.end(); - }); - if (it != directory_lookup_.end()) { - utils::collection::remove_element(it->second.handles, handle); - if (it->second.handles.empty()) { - directory_lookup_.erase(it); - } + auto iter = + std::find_if(directory_lookup_.begin(), directory_lookup_.end(), + [handle](auto &&item) -> bool { + auto &&handles = item.second.handles; + return std::find(handles.begin(), handles.end(), handle) != + item.second.handles.end(); + }); + if (iter == directory_lookup_.end()) { + return; + } + + utils::collection::remove_element(iter->second.handles, handle); + if (iter->second.handles.empty()) { + directory_lookup_.erase(iter); } } @@ -85,21 +89,26 @@ void directory_cache::service_function() { auto lookup = directory_lookup_; directory_lock.unlock(); - for (auto &&kv : lookup) { + for (const auto &item : lookup) { if (std::chrono::duration_cast( - std::chrono::system_clock::now() - kv.second.last_update) >= 120s) { + std::chrono::system_clock::now() - item.second.last_update) >= + 120s) { directory_lock.lock(); - directory_lookup_.erase(kv.first); + directory_lookup_.erase(item.first); directory_lock.unlock(); } } - if (not get_stop_requested()) { - unique_mutex_lock shutdown_lock(get_mutex()); - if (not get_stop_requested()) { - get_notify().wait_for(shutdown_lock, 15s); - } + if (get_stop_requested()) { + return; } + + unique_mutex_lock shutdown_lock(get_mutex()); + if (get_stop_requested()) { + return; + } + + get_notify().wait_for(shutdown_lock, 15s); } void directory_cache::set_directory( diff --git a/repertory/librepertory/src/drives/fuse/fuse_base.cpp b/repertory/librepertory/src/drives/fuse/fuse_base.cpp index 547dcf20..9b03f201 100644 --- a/repertory/librepertory/src/drives/fuse/fuse_base.cpp +++ b/repertory/librepertory/src/drives/fuse/fuse_base.cpp @@ -632,7 +632,7 @@ auto fuse_base::parse_args(std::vector &args) -> int { } const auto option_parts = utils::string::split(options, ',', true); - for (auto &&option : option_parts) { + for (const auto &option : option_parts) { if (option.find("gid") == 0) { const auto parts = utils::string::split(option, '=', true); if (parts.size() == 2u) { diff --git a/repertory/librepertory/src/drives/fuse/fuse_drive.cpp b/repertory/librepertory/src/drives/fuse/fuse_drive.cpp index 1c25d562..54b1a1c9 100644 --- a/repertory/librepertory/src/drives/fuse/fuse_drive.cpp +++ b/repertory/librepertory/src/drives/fuse/fuse_drive.cpp @@ -958,7 +958,7 @@ auto fuse_drive::listxattr_impl(std::string api_path, char *buffer, size_t size, api_meta_map meta; res = provider_.get_item_meta(api_path, meta); if (res == api_error::success) { - for (auto &&meta_item : meta) { + for (const auto &meta_item : meta) { if (utils::collection::excludes(META_USED_NAMES, meta_item.first)) { auto attribute_name = meta_item.first; #if defined(__APPLE__) diff --git a/repertory/librepertory/src/drives/remote/remote_open_file_table.cpp b/repertory/librepertory/src/drives/remote/remote_open_file_table.cpp index bb92cb72..c86604cb 100644 --- a/repertory/librepertory/src/drives/remote/remote_open_file_table.cpp +++ b/repertory/librepertory/src/drives/remote/remote_open_file_table.cpp @@ -65,7 +65,7 @@ void remote_open_file_table::close_all(const std::string &client_id) { }); lock.unlock(); - for (auto &&handle : compat_handles) { + for (const auto &handle : compat_handles) { #if defined(_WIN32) _close(static_cast(handle)); #else @@ -74,7 +74,7 @@ void remote_open_file_table::close_all(const std::string &client_id) { remove_compat_open_info(handle); } - for (auto &&handle : handles) { + for (const auto &handle : handles) { #if defined(_WIN32) ::CloseHandle(handle); #else // !defined(_WIN32) @@ -85,14 +85,14 @@ void remote_open_file_table::close_all(const std::string &client_id) { std::vector dirs; lock.lock(); - for (auto &&kv : directory_lookup_) { + for (const auto &kv : directory_lookup_) { if (kv.first == client_id) { dirs.insert(dirs.end(), kv.second.begin(), kv.second.end()); } } lock.unlock(); - for (auto &&dir : dirs) { + for (const auto &dir : dirs) { remove_directory(client_id, dir); } } @@ -185,11 +185,11 @@ void remote_open_file_table::remove_all(const std::string &file_path) { }); lock.unlock(); - for (auto &&handle : compat_open_list) { + for (const auto &handle : compat_open_list) { remove_compat_open_info(handle); } - for (auto &&handle : open_list) { + for (const auto &handle : open_list) { remove_open_info(handle); } } @@ -262,7 +262,7 @@ void remote_open_file_table::remove_and_close_all(const native_handle &handle) { auto op_info = *file_lookup_.at(handle_lookup_.at(handle)); lock.unlock(); - for (auto &&open_handle : op_info.handles) { + for (const auto &open_handle : op_info.handles) { #if defined(_WIN32) ::CloseHandle(open_handle); #else // !defined(_WIN32) diff --git a/repertory/librepertory/src/drives/winfsp/remotewinfsp/remote_winfsp_drive.cpp b/repertory/librepertory/src/drives/winfsp/remotewinfsp/remote_winfsp_drive.cpp index 2344d0c7..b62c0af9 100644 --- a/repertory/librepertory/src/drives/winfsp/remotewinfsp/remote_winfsp_drive.cpp +++ b/repertory/librepertory/src/drives/winfsp/remotewinfsp/remote_winfsp_drive.cpp @@ -238,7 +238,7 @@ auto remote_winfsp_drive::mount(const std::vector &drive_args) auto force_no_console = utils::collection::includes(drive_args, "-nc"); auto enable_console = false; - for (auto &&arg : drive_args) { + for (const auto &arg : drive_args) { if (arg == "-f") { if (not force_no_console) { enable_console = true; @@ -352,7 +352,7 @@ auto remote_winfsp_drive::ReadDirectory(PVOID /*file_node*/, PVOID file_desc, directory_buffer, static_cast(nullptr == marker), &ret)) { auto item_found = false; - for (auto &&item : item_list) { + for (const auto &item : item_list) { auto item_path = item["path"].get(); auto display_name = utils::string::from_utf8( utils::path::strip_to_file_name(item_path)); diff --git a/repertory/librepertory/src/drives/winfsp/winfsp_drive.cpp b/repertory/librepertory/src/drives/winfsp/winfsp_drive.cpp index 50d854ec..a8d24319 100644 --- a/repertory/librepertory/src/drives/winfsp/winfsp_drive.cpp +++ b/repertory/librepertory/src/drives/winfsp/winfsp_drive.cpp @@ -583,7 +583,7 @@ auto winfsp_drive::mount(const std::vector &drive_args) -> int { auto force_no_console = utils::collection::includes(drive_args, "-nc"); auto enable_console = false; - for (auto &&arg : drive_args) { + for (const auto &arg : drive_args) { if (arg == "-f") { if (not force_no_console) { enable_console = true; diff --git a/repertory/librepertory/src/utils/polling.cpp b/repertory/librepertory/src/utils/polling.cpp index 21e6b290..4252f832 100644 --- a/repertory/librepertory/src/utils/polling.cpp +++ b/repertory/librepertory/src/utils/polling.cpp @@ -146,7 +146,7 @@ void polling::stop() { notify_.notify_all(); thread_lock.unlock(); - for (auto &&thread : frequency_threads_) { + for (auto &thread : frequency_threads_) { thread->join(); thread.reset(); } diff --git a/repertory/repertory_test/src/fuse_drive_access_test.cpp b/repertory/repertory_test/src/fuse_drive_access_test.cpp index cc518c79..c141d5b5 100644 --- a/repertory/repertory_test/src/fuse_drive_access_test.cpp +++ b/repertory/repertory_test/src/fuse_drive_access_test.cpp @@ -103,7 +103,7 @@ TYPED_TEST(fuse_test, access_directory_permutations_test) { std::string dir_name{"access_test"}; auto dir_path = this->create_directory_and_test(dir_name); - for (auto &&permutation : access_permutations) { + for (const auto &permutation : access_permutations) { perform_access_test(permutation, dir_path); } @@ -114,7 +114,7 @@ TYPED_TEST(fuse_test, access_file_permutations_test) { std::string file_name{"access_test"}; auto file_path = this->create_file_and_test(file_name); - for (auto &&permutation : access_permutations) { + for (const auto &permutation : access_permutations) { perform_access_test(permutation, file_path); } diff --git a/repertory/repertory_test/src/fuse_drive_create_and_open_test.cpp b/repertory/repertory_test/src/fuse_drive_create_and_open_test.cpp index e1681d72..3c4405fa 100644 --- a/repertory/repertory_test/src/fuse_drive_create_and_open_test.cpp +++ b/repertory/repertory_test/src/fuse_drive_create_and_open_test.cpp @@ -41,7 +41,7 @@ TYPED_TEST(fuse_test, create_can_create_directory_with_specific_perms) { std::string dir_name{"create_test"}; auto dir_path = this->create_directory_and_test(dir_name, S_IRUSR); - struct stat64 unix_st {}; + struct stat64 unix_st{}; EXPECT_EQ(0, stat64(dir_path.c_str(), &unix_st)); EXPECT_EQ(S_IRUSR, unix_st.st_mode & ACCESSPERMS); @@ -52,7 +52,7 @@ TYPED_TEST(fuse_test, create_can_create_file_with_specific_perms) { std::string file_name{"create_test"}; auto file_path = this->create_file_and_test(file_name, S_IRUSR); - struct stat64 unix_st {}; + struct stat64 unix_st{}; EXPECT_EQ(0, stat64(file_path.c_str(), &unix_st)); EXPECT_EQ(S_IRUSR, unix_st.st_mode & ACCESSPERMS); @@ -376,7 +376,7 @@ TYPED_TEST(fuse_test, create_fails_with_excl_if_path_is_directory) { std::string dir_name{"create_test"}; auto dir_path = this->create_directory_and_test(dir_name); - for (auto &&flags : ops) { + for (const auto &flags : ops) { auto handle = open(dir_path.c_str(), flags, ACCESSPERMS); EXPECT_EQ(-1, handle); @@ -396,7 +396,7 @@ TYPED_TEST(fuse_test, create_fails_with_excl_if_file_exists) { std::string file_name{"create_test"}; auto file_path = this->create_file_and_test(file_name); - for (auto &&flags : ops) { + for (const auto &flags : ops) { auto handle = open(file_path.c_str(), flags, ACCESSPERMS); EXPECT_EQ(-1, handle); @@ -420,7 +420,7 @@ TYPED_TEST(fuse_test, create_fails_if_path_is_directory) { std::string dir_name{"create_test"}; auto dir_path = this->create_directory_and_test(dir_name); - for (auto &&flags : ops) { + for (const auto &flags : ops) { auto handle = open(dir_path.c_str(), flags, ACCESSPERMS); EXPECT_EQ(-1, handle); @@ -447,7 +447,7 @@ TYPED_TEST(fuse_test, create_fails_if_parent_path_does_not_exist) { std::string file_name{"no_dir/create_test"}; auto file_path = this->create_file_path(file_name); - for (auto &&flags : ops) { + for (const auto &flags : ops) { auto handle = open(file_path.c_str(), flags, ACCESSPERMS); EXPECT_EQ(-1, handle); @@ -463,7 +463,7 @@ TYPED_TEST(fuse_test, create_fails_if_invalid) { std::string file_name{"create_test"}; auto file_path = this->create_file_path(file_name); - for (auto &&flags : ops) { + for (const auto &flags : ops) { auto handle = open(file_path.c_str(), flags, ACCESSPERMS); EXPECT_EQ(-1, handle); @@ -481,7 +481,7 @@ TYPED_TEST(fuse_test, create_open_fails_if_path_is_directory) { std::string dir_name{"create_test"}; auto dir_path = this->create_directory_and_test(dir_name); - for (auto &&flags : ops) { + for (const auto &flags : ops) { auto handle = open(dir_path.c_str(), flags); EXPECT_EQ(-1, handle); if (handle != -1) { @@ -510,7 +510,7 @@ TYPED_TEST(fuse_test, create_open_fails_if_path_does_not_exist) { std::string file_name{"create_test"}; auto file_path = this->create_file_path(file_name); - for (auto &&flags : ops) { + for (const auto &flags : ops) { auto handle = open(file_path.c_str(), flags); EXPECT_EQ(-1, handle); EXPECT_EQ(ENOENT, errno); diff --git a/repertory/repertory_test/src/winfsp_drive_create_test.cpp b/repertory/repertory_test/src/winfsp_drive_create_test.cpp index f57819b2..0c9befdb 100644 --- a/repertory/repertory_test/src/winfsp_drive_create_test.cpp +++ b/repertory/repertory_test/src/winfsp_drive_create_test.cpp @@ -97,7 +97,7 @@ TYPED_TEST(winfsp_test, cr8_file_can_delete_file_after_close) { TYPED_TEST(winfsp_test, cr8_file_cannot_create_files_with_invalid_characters_in_path) { - for (auto &&invalid_char : std::array{ + for (const auto &invalid_char : std::array{ {"*", ":", "<", ">", "?", "|", "\""}, }) { auto handle = ::CreateFileA(