Implement recursive directory deletion.

runtime/bin/directory_posix.cc

Index: runtime/bin/directory_posix.cc
diff --git a/runtime/bin/directory_posix.cc b/runtime/bin/directory_posix.cc
index c201988325b4052ea2bc21e6ddec98377d1036a4..0bcc62b242f9c4ccf32405c65ec601977ab532db 100644
--- a/runtime/bin/directory_posix.cc
+++ b/runtime/bin/directory_posix.cc
@@ -27,13 +27,14 @@ static void SetOsErrorMessage(char* os_error_message,
 }
 
 
-// Forward declaration.
+// Forward declarations.
 static bool ListRecursively(const char* dir_name,
                             bool recursive,
                             Dart_Port dir_port,
                             Dart_Port file_port,
                             Dart_Port done_port,
                             Dart_Port error_port);
+static bool DeleteRecursively(const char* dir_name);
 
 
 static void ComputeFullPath(const char* dir_name,
@@ -220,6 +221,106 @@ static bool ListRecursively(const char* dir_name,
 }
 
 
+static bool DeleteFile(char* file_name,

[Bill Hesse, 2012/02/02 17:24:39]

Put a comment that strlen(path) may not be path_length, but that path_length is
the valid part of path?

[Mads Ager (google), 2012/02/03 11:48:54]

This applies to all the methods in this file. I have added a comment to that
effect in the two *Recursively methods.

+                       char* path,
+                       int path_length) {
+  size_t written = snprintf(path + path_length,
+                            PATH_MAX - path_length,
+                            "%s",
+                            file_name);
+  ASSERT(written == strlen(file_name));
+  return (remove(path) == 0);
+}
+
+
+static bool DeleteDir(char* dir_name,
+                      char* path,
+                      int path_length) {
+  if (strcmp(dir_name, ".") != 0 &&
+      strcmp(dir_name, "..") != 0) {
+    size_t written = snprintf(path + path_length,
+                              PATH_MAX - path_length,
+                              "%s",
+                              dir_name);
+    ASSERT(written == strlen(dir_name));

[Bill Hesse, 2012/02/02 17:24:39]

In release mode, could a snprintf turn "long_path/..funny_directory" into
"long_path/..", and get it to DeleteRecursively?  It seems unlikely, but I'm not
seeing anything that makes it impossible.

[Mads Ager (google), 2012/02/03 11:48:54]

Yes, that is not good. I have changed all the ASSERTS that manipulate the path
for listing and recursive deletion to explicit checks instead.

+    return DeleteRecursively(path);
+  }
+  return true;
+}
+
+
+static bool DeleteRecursively(const char* dir_name) {
+  DIR* dir_pointer;
+  do {
+    dir_pointer = opendir(dir_name);
+  } while (dir_pointer == NULL && errno == EINTR);
+
+  if (dir_pointer == NULL) {
+    return false;
+  }
+
+  // Compute full path for the directory currently being deleted.
+  char *path = static_cast<char*>(malloc(PATH_MAX));
+  ASSERT(path != NULL);
+  int path_length = 0;
+  ComputeFullPath(dir_name, path, &path_length);

[Bill Hesse, 2012/02/02 17:24:39]

ASSERT on path_length here?

[Mads Ager (google), 2012/02/03 11:48:54]

Reworked so ComputeFullPath returns a failure status if it could not compute the
full path reliably.

+
+  // Iterated the directory and delete all files and directories.

[Bill Hesse, 2012/02/02 17:24:39]

Iterate the directory ...

[Mads Ager (google), 2012/02/03 11:48:54]

Done.

+  int success = 0;
+  bool error = false;
+  dirent entry;
+  dirent* result;
+  while ((success = TEMP_FAILURE_RETRY(readdir_r(dir_pointer,
+                                                 &entry,
+                                                 &result))) == 0 &&
+         result != NULL &&
+         !error) {
+    switch (entry.d_type) {
+      case DT_DIR:
+        error = error || !DeleteDir(entry.d_name, path, path_length);
+        break;
+      case DT_REG:
+        error = error || !DeleteFile(entry.d_name, path, path_length);
+        break;
+      case DT_UNKNOWN: {
+        // On some file systems the entry type is not determined by
+        // readdir_r. For those we use lstat to determine the entry
+        // type.
+        struct stat entry_info;
+        size_t written = snprintf(path + path_length,
+                                  PATH_MAX - path_length,
+                                  "%s",
+                                  entry.d_name);
+        ASSERT(written == strlen(entry.d_name));
+        int lstat_success = TEMP_FAILURE_RETRY(lstat(path, &entry_info));
+        if (lstat_success == -1) {
+          error = true;
+          break;
+        }
+        if ((entry_info.st_mode & S_IFMT) == S_IFDIR) {
+          error = error || !DeleteDir(entry.d_name, path, path_length);
+        } else if ((entry_info.st_mode & S_IFMT) == S_IFREG) {
+          error = error || !DeleteFile(entry.d_name, path, path_length);
+        }
+        break;
+      }
+      default:
+        break;
+    }
+  }
+
+  free(path);
+
+  if ((success != 0) ||
+      (closedir(dir_pointer) == -1) ||
+      (remove(dir_name) == -1)) {
+    return false;
+  }
+
+  return !error;

[Bill Hesse, 2012/02/02 17:24:39]

Every use of error really wants to use !error instead.  Is there a good name for
!error, so the sense could be reversed?

[Mads Ager (google), 2012/02/03 11:48:54]

Good point. Renamed success->read, error->success and reversed the sense.

+}
+
+
 void Directory::List(const char* dir_name,
                      bool recursive,
                      Dart_Port dir_port,
@@ -309,6 +410,10 @@ int Directory::CreateTemp(const char* const_template,
 }
 
 
-bool Directory::Delete(const char* dir_name) {
-  return (TEMP_FAILURE_RETRY(rmdir(dir_name)) == 0);
+bool Directory::Delete(const char* dir_name, bool recursive) {
+  if (!recursive) {
+    return (TEMP_FAILURE_RETRY(remove(dir_name)) == 0);
+  } else {
+    return DeleteRecursively(dir_name);
+  }
 }