From b3cfcf55ead41d0458f83d26d7258a1701356e3b Mon Sep 17 00:00:00 2001 From: archshift Date: Thu, 26 Nov 2015 00:34:26 -0800 Subject: Refactor ScanDirectoryTreeAndCallback to separate errors and retvals ScanDirectoryTreeAndCallback, before this change, coupled error/return codes and actual return values (number of entries found). This caused confusion and difficulty interpreting the precise way the function worked. Supersedes, and closes #1255. --- src/citra_qt/game_list.cpp | 16 ++++++----- src/common/file_util.cpp | 72 ++++++++++++++++++++++------------------------ src/common/file_util.h | 31 +++++++++++--------- 3 files changed, 62 insertions(+), 57 deletions(-) diff --git a/src/citra_qt/game_list.cpp b/src/citra_qt/game_list.cpp index e925f08a7..1f8d69a03 100644 --- a/src/citra_qt/game_list.cpp +++ b/src/citra_qt/game_list.cpp @@ -119,13 +119,14 @@ void GameList::LoadInterfaceLayout(QSettings& settings) void GameListWorker::AddFstEntriesToGameList(const std::string& dir_path, bool deep_scan) { - const auto callback = [&](const std::string& directory, - const std::string& virtual_name) -> int { + const auto callback = [&](unsigned* num_entries_out, + const std::string& directory, + const std::string& virtual_name) -> bool { std::string physical_name = directory + DIR_SEP + virtual_name; if (stop_processing) - return -1; // A negative return value breaks the callback loop. + return false; // Breaks the callback loop. if (deep_scan && FileUtil::IsDirectory(physical_name)) { AddFstEntriesToGameList(physical_name, true); @@ -135,11 +136,11 @@ void GameListWorker::AddFstEntriesToGameList(const std::string& dir_path, bool d Loader::FileType guessed_filetype = Loader::GuessFromExtension(filename_extension); if (guessed_filetype == Loader::FileType::Unknown) - return 0; + return true; Loader::FileType filetype = Loader::IdentifyFile(physical_name); if (filetype == Loader::FileType::Unknown) { LOG_WARNING(Frontend, "File %s is of indeterminate type and is possibly corrupted.", physical_name.c_str()); - return 0; + return true; } if (guessed_filetype != filetype) { LOG_WARNING(Frontend, "Filetype and extension of file %s do not match.", physical_name.c_str()); @@ -152,9 +153,10 @@ void GameListWorker::AddFstEntriesToGameList(const std::string& dir_path, bool d }); } - return 0; // We don't care about the found entries + return true; }; - FileUtil::ScanDirectoryTreeAndCallback(dir_path, callback); + + FileUtil::ForeachDirectoryEntry(nullptr, dir_path, callback); } void GameListWorker::run() diff --git a/src/common/file_util.cpp b/src/common/file_util.cpp index 1e0d33313..4c7113390 100644 --- a/src/common/file_util.cpp +++ b/src/common/file_util.cpp @@ -420,11 +420,13 @@ bool CreateEmptyFile(const std::string &filename) } -int ScanDirectoryTreeAndCallback(const std::string &directory, std::function callback) +bool ForeachDirectoryEntry(unsigned* num_entries_out, const std::string &directory, DirectoryEntryCallable callback) { LOG_TRACE(Common_Filesystem, "directory %s", directory.c_str()); + // How many files + directories we found - int found_entries = 0; + unsigned found_entries = 0; + #ifdef _WIN32 // Find the first file in the directory. WIN32_FIND_DATA ffd; @@ -432,7 +434,7 @@ int ScanDirectoryTreeAndCallback(const std::string &directory, std::functiond_name); #endif - // check for "." and ".." - if (((virtual_name[0] == '.') && (virtual_name[1] == '\0')) || - ((virtual_name[0] == '.') && (virtual_name[1] == '.') && - (virtual_name[2] == '\0'))) + + if (virtual_name == "." || virtual_name == "..") continue; - int ret = callback(directory, virtual_name); - if (ret < 0) { - if (ret != -1) - found_entries = ret; + unsigned ret_entries; + if (!callback(&ret_entries, directory, virtual_name)) break; - } - found_entries += ret; + found_entries += ret_entries; #ifdef _WIN32 } while (FindNextFile(handle_find, &ffd) != 0); @@ -469,16 +466,18 @@ int ScanDirectoryTreeAndCallback(const std::string &directory, std::function int { + const auto callback = [&parent_entry](unsigned* num_entries_out, + const std::string& directory, + const std::string& virtual_name) -> bool { FSTEntry entry; - int found_entries = 0; entry.virtualName = virtual_name; entry.physicalName = directory + DIR_SEP + virtual_name; @@ -486,41 +485,40 @@ int ScanDirectoryTree(const std::string &directory, FSTEntry& parent_entry) entry.isDirectory = true; // is a directory, lets go inside entry.size = ScanDirectoryTree(entry.physicalName, entry); - found_entries += (int)entry.size; + *num_entries_out += (int)entry.size; } else { // is a file entry.isDirectory = false; entry.size = GetSize(entry.physicalName); } - ++found_entries; + (*num_entries_out)++; + // Push into the tree parent_entry.children.push_back(entry); - return found_entries; + return true; }; - return ScanDirectoryTreeAndCallback(directory, callback); + unsigned num_entries; + return ForeachDirectoryEntry(&num_entries, directory, callback) ? num_entries : 0; } bool DeleteDirRecursively(const std::string &directory) { - const static auto callback = [](const std::string& directory, - const std::string& virtual_name) -> int { + const static auto callback = [](unsigned* num_entries_out, + const std::string& directory, + const std::string& virtual_name) -> bool { std::string new_path = directory + DIR_SEP_CHR + virtual_name; - if (IsDirectory(new_path)) { - if (!DeleteDirRecursively(new_path)) { - return -2; - } - } else if (!Delete(new_path)) { - return -2; - } - return 0; + if (IsDirectory(new_path)) + return DeleteDirRecursively(new_path); + + return Delete(new_path); }; - if (ScanDirectoryTreeAndCallback(directory, callback) == -2) { + if (!ForeachDirectoryEntry(nullptr, directory, callback)) return false; - } - FileUtil::DeleteDir(directory); + // Delete the outermost directory + FileUtil::DeleteDir(directory); return true; } diff --git a/src/common/file_util.h b/src/common/file_util.h index 3d617f573..a85121aa6 100644 --- a/src/common/file_util.h +++ b/src/common/file_util.h @@ -98,19 +98,24 @@ bool Copy(const std::string &srcFilename, const std::string &destFilename); bool CreateEmptyFile(const std::string &filename); /** - * Scans the directory tree, calling the callback for each file/directory found. - * The callback must return the number of files and directories which the provided path contains. - * If the callback's return value is -1, the callback loop is broken immediately. - * If the callback's return value is otherwise negative, the callback loop is broken immediately - * and the callback's return value is returned from this function (to allow for error handling). - * @param directory the parent directory to start scanning from - * @param callback The callback which will be called for each file/directory. It is called - * with the arguments (const std::string& directory, const std::string& virtual_name). - * The `directory `parameter is the path to the directory which contains the file/directory. - * The `virtual_name` parameter is the incomplete file path, without any directory info. - * @return the total number of files/directories found + * @param num_entries_out to be assigned by the callable with the number of iterated directory entries, never null + * @param directory the path to the enclosing directory + * @param virtual_name the entry name, without any preceding directory info + * @return whether handling the entry succeeded + */ +using DirectoryEntryCallable = std::function; + +/** + * Scans a directory, calling the callback for each file/directory contained within. + * If the callback returns failure, scanning halts and this function returns failure as well + * @param num_entries_out assigned by the function with the number of iterated directory entries, can be null + * @param directory the directory to scan + * @param callback The callback which will be called for each entry + * @return whether scanning the directory succeeded */ -int ScanDirectoryTreeAndCallback(const std::string &directory, std::function callback); +bool ForeachDirectoryEntry(unsigned* num_entries_out, const std::string &directory, DirectoryEntryCallable callback); /** * Scans the directory tree, storing the results. @@ -118,7 +123,7 @@ int ScanDirectoryTreeAndCallback(const std::string &directory, std::function