Replace `rm -rf` with more portable implementation.

[vharron]

LGTM

[clayborg]

See inlined comments for needed changes.

lldb/trunk/source/Host/posix/FileSystem.cpp

85–103 ↗

(On Diff #28635)

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);

[clayborg]

Ah, I missed that this was committed. I will fix this.

[chaoren]

Please let me fix this. I need to redeem myself.

[clayborg]

I implemented my suggestions with:

% svn commit
Sending        include/lldb/Host/FileSpec.h
Sending        source/Host/common/FileSpec.cpp
Sending        source/Host/posix/FileSystem.cpp
Transmitting file data ...
Committed revision 240978.

[clayborg]

Sorry I didn't see this email before I fixed it.

[chaoren]

It's okay, it was my fault to begin with.

source/Host/posix/FileSystem.cpp

 //===-- FileSystem.cpp ------------------------------------------*- C++ -*-===//
 //
 //                     The LLVM Compiler Infrastructure
 //
 // This file is distributed under the University of Illinois Open Source
 // License. See LICENSE.TXT for details.
 //
 //===----------------------------------------------------------------------===//
 
 #include "lldb/Host/FileSystem.h"
 
 // C includes
+#include <dirent.h>
 #include <sys/mount.h>
 #include <sys/param.h>
 #include <sys/stat.h>
 #include <sys/types.h>
 #ifdef __linux__
 #include <sys/statfs.h>
 #include <sys/mount.h>
 #include <linux/magic.h>

 Error
 FileSystem::DeleteDirectory(const FileSpec &file_spec, bool recurse)
 {
     Error error;
     if (file_spec)
     {
         if (recurse)
         {
-            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);
         }
         else
         {
             if (::rmdir(file_spec.GetCString()) != 0)
                 error.SetErrorToErrno();
         }
     }
     else