[Lldb-commits] [PATCH] Replace `rm -rf` with more portable implementation.

[Greg Clayton]

See inlined comments for needed changes.


REPOSITORY
  rL LLVM

================
Comment at: lldb/trunk/source/Host/posix/FileSystem.cpp:85-103
@@ -83,7 +84,21 @@
         {
-            StreamString command;
-            command.Printf("rm -rf \"%s\"", file_spec.GetCString());
-            int status = ::system(command.GetString().c_str());
-            if (status != 0)
-                error.SetError(status, eErrorTypeGeneric);
+            DIR *dirp = opendir(file_spec.GetCString());
+            if (!dirp)
+            {
+                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();
+            }
+            if (closedir(dirp) != 0)
+                error.SetErrorToErrno();
+            if (error.Fail())
+                return error;
+            return DeleteDirectory(file_spec, false);
         }
----------------
Please fix the following things:

1 - use FileSpec::EnumerateDirectory(...) instead of using opendir(), readdir(), closedir() directly
2 - Save the directories up in a std::vector and process them _after_ the call the FileSpec::EnumerateDirectory(...) has completed iterating through the current directory so we don't run out of file descriptors when we try to delete a directory that has 1000000 entries inside of it. Each opendir()/closedir() takes up a file descriptor and if you recursively call this function you will run out of fds. You want to avoid the recursive nature here and only use 1 file descriptor at a time for the recursion by just saving the list of directories to delete, then iterating through those directories and calling FileSystem::DeleteDirectory() with each item.

Extra credit for possibly making a new version of FileSpec::EnumerateDirectory(...) that uses a lambda instead of a callback. Something like:

typedef std::function < FileSpec::EnumerateDirectoryResult(FileType file_type, const FileSpec &spec)> FileCallback;

FileSpec::EnumerateDirectoryResult
FileSpec::ForEachDirectory (const char *dir_path, 
    bool find_directories,
    bool find_files,
    bool find_other,
    FileCallback const& callback);

http://reviews.llvm.org/D10787

EMAIL PREFERENCES
  http://reviews.llvm.org/settings/panel/emailpreferences/