[Lldb-commits] [lldb] r240978 - Avoid a recursive function call that could run LLDB out of file descriptors in FileSystem::DeleteDirectory(...).
Greg Clayton
gclayton at apple.com
Mon Jun 29 11:29:00 PDT 2015
Author: gclayton
Date: Mon Jun 29 13:29:00 2015
New Revision: 240978
URL: http://llvm.org/viewvc/llvm-project?rev=240978&view=rev
Log:
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
Modified:
lldb/trunk/include/lldb/Host/FileSpec.h
lldb/trunk/source/Host/common/FileSpec.cpp
lldb/trunk/source/Host/posix/FileSystem.cpp
Modified: lldb/trunk/include/lldb/Host/FileSpec.h
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Host/FileSpec.h?rev=240978&r1=240977&r2=240978&view=diff
==============================================================================
--- lldb/trunk/include/lldb/Host/FileSpec.h (original)
+++ lldb/trunk/include/lldb/Host/FileSpec.h Mon Jun 29 13:29:00 2015
@@ -812,6 +812,11 @@ public:
EnumerateDirectoryCallbackType callback,
void *callback_baton);
+ typedef std::function <EnumerateDirectoryResult(FileType file_type, const FileSpec &spec)> DirectoryCallback;
+
+ static EnumerateDirectoryResult
+ ForEachItemInDirectory (const char *dir_path, DirectoryCallback const &callback);
+
protected:
//------------------------------------------------------------------
// Member variables
Modified: lldb/trunk/source/Host/common/FileSpec.cpp
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Host/common/FileSpec.cpp?rev=240978&r1=240977&r2=240978&view=diff
==============================================================================
--- lldb/trunk/source/Host/common/FileSpec.cpp (original)
+++ lldb/trunk/source/Host/common/FileSpec.cpp Mon Jun 29 13:29:00 2015
@@ -1070,6 +1070,198 @@ FileSpec::ReadFileLines (STLStringArray
}
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 *, int> 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
(
const char *dir_path,
Modified: lldb/trunk/source/Host/posix/FileSystem.cpp
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Host/posix/FileSystem.cpp?rev=240978&r1=240977&r2=240978&view=diff
==============================================================================
--- lldb/trunk/source/Host/posix/FileSystem.cpp (original)
+++ lldb/trunk/source/Host/posix/FileSystem.cpp Mon Jun 29 13:29:00 2015
@@ -82,27 +82,43 @@ FileSystem::DeleteDirectory(const FileSp
{
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<FileSpec> 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;
- }
- 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();
+ // 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;
+ }
}
- 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();
More information about the lldb-commits
mailing list