From 58c65f0243c5118716180dbb50ea7d52c79a1e90 Mon Sep 17 00:00:00 2001 From: Greg Clayton Date: Mon, 29 Jun 2015 18:29:00 +0000 Subject: [PATCH] Avoid a recursive function call that could run LLDB out of file descriptors in FileSystem::DeleteDirectory(...). Fixes include: - use FileSystem::Unlink() instead of a direct call to ::unlink(...) when deleting files when iterating through the current directory - save directories from current directory in a list and iterate through those _after_ the current directory has been iterated - Use new FileSpec::ForEachItemInDirectory() instead of manually iterating across directories with opendir()/readdir()/closedir() We should switch all code over to using FileSpec::ForEachItemInDirectory(...) in the near future and get rid of FileSpec::EnumerateDirectory(). This is a follow up patch to: http://reviews.llvm.org/D10787 llvm-svn: 240978 --- lldb/include/lldb/Host/FileSpec.h | 5 + lldb/source/Host/common/FileSpec.cpp | 192 ++++++++++++++++++++++++++ lldb/source/Host/posix/FileSystem.cpp | 52 ++++--- 3 files changed, 231 insertions(+), 18 deletions(-) diff --git a/lldb/include/lldb/Host/FileSpec.h b/lldb/include/lldb/Host/FileSpec.h index d5e88074a879..5aa999096635 100644 --- a/lldb/include/lldb/Host/FileSpec.h +++ b/lldb/include/lldb/Host/FileSpec.h @@ -812,6 +812,11 @@ public: EnumerateDirectoryCallbackType callback, void *callback_baton); + typedef std::function DirectoryCallback; + + static EnumerateDirectoryResult + ForEachItemInDirectory (const char *dir_path, DirectoryCallback const &callback); + protected: //------------------------------------------------------------------ // Member variables diff --git a/lldb/source/Host/common/FileSpec.cpp b/lldb/source/Host/common/FileSpec.cpp index d988b7f43506..0cdc0730238b 100644 --- a/lldb/source/Host/common/FileSpec.cpp +++ b/lldb/source/Host/common/FileSpec.cpp @@ -1069,6 +1069,198 @@ FileSpec::ReadFileLines (STLStringArray &lines) return lines.size(); } +FileSpec::EnumerateDirectoryResult +FileSpec::ForEachItemInDirectory (const char *dir_path, DirectoryCallback const &callback) +{ + if (dir_path && dir_path[0]) + { +#if _WIN32 + std::string szDir(dir_path); + szDir += "\\*"; + + WIN32_FIND_DATA ffd; + HANDLE hFind = FindFirstFile(szDir.c_str(), &ffd); + + if (hFind == INVALID_HANDLE_VALUE) + { + return eEnumerateDirectoryResultNext; + } + + do + { + bool call_callback = false; + FileSpec::FileType file_type = eFileTypeUnknown; + if (ffd.dwFileAttributes & FILE_ATTRIBUTE_DIRECTORY) + { + size_t len = strlen(ffd.cFileName); + + if (len == 1 && ffd.cFileName[0] == '.') + continue; + + if (len == 2 && ffd.cFileName[0] == '.' && ffd.cFileName[1] == '.') + continue; + + file_type = eFileTypeDirectory; + call_callback = find_directories; + } + else if (ffd.dwFileAttributes & FILE_ATTRIBUTE_DEVICE) + { + file_type = eFileTypeOther; + call_callback = find_other; + } + else + { + file_type = eFileTypeRegular; + call_callback = find_files; + } + if (call_callback) + { + char child_path[MAX_PATH]; + const int child_path_len = ::snprintf (child_path, sizeof(child_path), "%s\\%s", dir_path, ffd.cFileName); + if (child_path_len < (int)(sizeof(child_path) - 1)) + { + // Don't resolve the file type or path + FileSpec child_path_spec (child_path, false); + + EnumerateDirectoryResult result = callback (file_type, child_path_spec); + + switch (result) + { + case eEnumerateDirectoryResultNext: + // Enumerate next entry in the current directory. We just + // exit this switch and will continue enumerating the + // current directory as we currently are... + break; + + case eEnumerateDirectoryResultEnter: // Recurse into the current entry if it is a directory or symlink, or next if not + if (FileSpec::ForEachItemInDirectory(child_path, callback) == eEnumerateDirectoryResultQuit) + { + // The subdirectory returned Quit, which means to + // stop all directory enumerations at all levels. + return eEnumerateDirectoryResultQuit; + } + break; + + case eEnumerateDirectoryResultExit: // Exit from the current directory at the current level. + // Exit from this directory level and tell parent to + // keep enumerating. + return eEnumerateDirectoryResultNext; + + case eEnumerateDirectoryResultQuit: // Stop directory enumerations at any level + return eEnumerateDirectoryResultQuit; + } + } + } + } while (FindNextFile(hFind, &ffd) != 0); + + FindClose(hFind); +#else + lldb_utility::CleanUp dir_path_dir(opendir(dir_path), NULL, closedir); + if (dir_path_dir.is_valid()) + { + char dir_path_last_char = dir_path[strlen(dir_path) - 1]; + + long path_max = fpathconf (dirfd (dir_path_dir.get()), _PC_NAME_MAX); +#if defined (__APPLE_) && defined (__DARWIN_MAXPATHLEN) + if (path_max < __DARWIN_MAXPATHLEN) + path_max = __DARWIN_MAXPATHLEN; +#endif + struct dirent *buf, *dp; + buf = (struct dirent *) malloc (offsetof (struct dirent, d_name) + path_max + 1); + + while (buf && readdir_r(dir_path_dir.get(), buf, &dp) == 0 && dp) + { + // Only search directories + if (dp->d_type == DT_DIR || dp->d_type == DT_UNKNOWN) + { + size_t len = strlen(dp->d_name); + + if (len == 1 && dp->d_name[0] == '.') + continue; + + if (len == 2 && dp->d_name[0] == '.' && dp->d_name[1] == '.') + continue; + } + + FileSpec::FileType file_type = eFileTypeUnknown; + + switch (dp->d_type) + { + default: + case DT_UNKNOWN: file_type = eFileTypeUnknown; break; + case DT_FIFO: file_type = eFileTypePipe; break; + case DT_CHR: file_type = eFileTypeOther; break; + case DT_DIR: file_type = eFileTypeDirectory; break; + case DT_BLK: file_type = eFileTypeOther; break; + case DT_REG: file_type = eFileTypeRegular; break; + case DT_LNK: file_type = eFileTypeSymbolicLink; break; + case DT_SOCK: file_type = eFileTypeSocket; break; +#if !defined(__OpenBSD__) + case DT_WHT: file_type = eFileTypeOther; break; +#endif + } + + char child_path[PATH_MAX]; + + // Don't make paths with "/foo//bar", that just confuses everybody. + int child_path_len; + if (dir_path_last_char == '/') + child_path_len = ::snprintf (child_path, sizeof(child_path), "%s%s", dir_path, dp->d_name); + else + child_path_len = ::snprintf (child_path, sizeof(child_path), "%s/%s", dir_path, dp->d_name); + + if (child_path_len < (int)(sizeof(child_path) - 1)) + { + // Don't resolve the file type or path + FileSpec child_path_spec (child_path, false); + + EnumerateDirectoryResult result = callback (file_type, child_path_spec); + + switch (result) + { + case eEnumerateDirectoryResultNext: + // Enumerate next entry in the current directory. We just + // exit this switch and will continue enumerating the + // current directory as we currently are... + break; + + case eEnumerateDirectoryResultEnter: // Recurse into the current entry if it is a directory or symlink, or next if not + if (FileSpec::ForEachItemInDirectory (child_path, callback) == eEnumerateDirectoryResultQuit) + { + // The subdirectory returned Quit, which means to + // stop all directory enumerations at all levels. + if (buf) + free (buf); + return eEnumerateDirectoryResultQuit; + } + break; + + case eEnumerateDirectoryResultExit: // Exit from the current directory at the current level. + // Exit from this directory level and tell parent to + // keep enumerating. + if (buf) + free (buf); + return eEnumerateDirectoryResultNext; + + case eEnumerateDirectoryResultQuit: // Stop directory enumerations at any level + if (buf) + free (buf); + return eEnumerateDirectoryResultQuit; + } + } + } + if (buf) + { + free (buf); + } + } +#endif + } + // By default when exiting a directory, we tell the parent enumeration + // to continue enumerating. + return eEnumerateDirectoryResultNext; +} + FileSpec::EnumerateDirectoryResult FileSpec::EnumerateDirectory ( diff --git a/lldb/source/Host/posix/FileSystem.cpp b/lldb/source/Host/posix/FileSystem.cpp index 1ec53e2cc661..52698039b46e 100644 --- a/lldb/source/Host/posix/FileSystem.cpp +++ b/lldb/source/Host/posix/FileSystem.cpp @@ -82,27 +82,43 @@ FileSystem::DeleteDirectory(const FileSpec &file_spec, bool recurse) { if (recurse) { - DIR *dirp = opendir(file_spec.GetCString()); - if (!dirp) + // Save all sub directories in a list so we don't recursively call this function + // and possibly run out of file descriptors if the directory is too deep. + std::vector sub_directories; + + FileSpec::ForEachItemInDirectory (file_spec.GetCString(), [&error, &sub_directories](FileSpec::FileType file_type, const FileSpec &spec) -> FileSpec::EnumerateDirectoryResult { + if (file_type == FileSpec::eFileTypeDirectory) + { + // Save all directorires and process them after iterating through this directory + sub_directories.push_back(spec); + } + else + { + // Update sub_spec to point to the current file and delete it + error = FileSystem::Unlink(spec); + } + // If anything went wrong, stop iterating, else process the next file + if (error.Fail()) + return FileSpec::eEnumerateDirectoryResultQuit; + else + return FileSpec::eEnumerateDirectoryResultNext; + }); + + if (error.Success()) { - error.SetErrorToErrno(); - return error; + // Now delete all sub directories with separate calls that aren't + // recursively calling into this function _while_ this function is + // iterating through the current directory. + for (const auto &sub_directory : sub_directories) + { + error = DeleteDirectory(sub_directory, recurse); + if (error.Fail()) + break; + } } - struct dirent *direntp; - while (error.Success() && (direntp = readdir(dirp))) - { - if (direntp->d_type == DT_DIR) - error = DeleteDirectory(FileSpec{direntp->d_name, false}, true); - else if (::unlink(direntp->d_name) != 0) - error.SetErrorToErrno(); - } - if (closedir(dirp) != 0) - error.SetErrorToErrno(); - if (error.Fail()) - return error; - return DeleteDirectory(file_spec, false); } - else + + if (error.Success()) { if (::rmdir(file_spec.GetCString()) != 0) error.SetErrorToErrno();