Fixes some memory leaks in //runtime/bin

runtime/bin/directory_android.cc

Index: runtime/bin/directory_android.cc
diff --git a/runtime/bin/directory_android.cc b/runtime/bin/directory_android.cc
index 40bcde3f64f431c80f2b4bc12f79243371c7ee00..53762c77d7c535a601de3ab8559acd11df4ff68a 100644
--- a/runtime/bin/directory_android.cc
+++ b/runtime/bin/directory_android.cc
@@ -14,34 +14,41 @@
 #include <sys/stat.h>  // NOLINT
 #include <unistd.h>  // NOLINT
 
+#include "bin/dartutils.h"
 #include "bin/file.h"
 #include "bin/platform.h"
-
 #include "platform/signal_blocker.h"
 
-
 namespace dart {
 namespace bin {
 
-
 PathBuffer::PathBuffer() : length_(0) {
-  data_ = calloc(PATH_MAX + 1,  sizeof(char));  // NOLINT
+  data_ = calloc(PATH_MAX + 1, sizeof(char));  // NOLINT

[Ivan Posva, 2016/03/13 20:59:19]

Please add a comment where this is being free'd.

[zra, 2016/03/14 18:07:52]

Moved the destructor from the header file into the source files so that the free
is right next to the calloc.

 }
 
+
 bool PathBuffer::AddW(const wchar_t* name) {
   UNREACHABLE();
   return false;
 }
 
+
 char* PathBuffer::AsString() const {
   return reinterpret_cast<char*>(data_);
 }
 
+
 wchar_t* PathBuffer::AsStringW() const {
   UNREACHABLE();
   return NULL;
 }
 
+
+const char* PathBuffer::AsScopedString() const {
+  return DartUtils::ScopedCopyCString(AsString());
+}
+
+
 bool PathBuffer::Add(const char* name) {
   char* data = AsString();
   int written = snprintf(data + length_,
@@ -49,9 +56,9 @@ bool PathBuffer::Add(const char* name) {
                          "%s",
                          name);
   data[PATH_MAX] = '\0';
-  if (written <= PATH_MAX - length_ &&
-      written >= 0 &&
-      static_cast<size_t>(written) == strnlen(name, PATH_MAX + 1)) {
+  if ((written <= PATH_MAX - length_) &&
+      (written >= 0) &&
+      (static_cast<size_t>(written) == strnlen(name, PATH_MAX + 1))) {
     length_ += written;
     return true;
   } else {
@@ -60,7 +67,8 @@ bool PathBuffer::Add(const char* name) {
   }
 }
 
-void PathBuffer::Reset(int new_length) {
+
+void PathBuffer::Reset(intptr_t new_length) {
   length_ = new_length;
   AsString()[length_] = '\0';
 }
@@ -84,7 +92,7 @@ ListType DirectoryListingEntry::Next(DirectoryListing* listing) {
     do {
       lister_ = reinterpret_cast<intptr_t>(
           opendir(listing->path_buffer().AsString()));
-    } while (lister_ == 0 && errno == EINTR);
+    } while ((lister_ == 0) && (errno == EINTR));
 
     if (lister_ == 0) {
       done_ = true;
@@ -106,18 +114,19 @@ ListType DirectoryListingEntry::Next(DirectoryListing* listing) {
   int status = 0;
   dirent entry;
   dirent* result;
-  if ((status = NO_RETRY_EXPECTED(readdir_r(reinterpret_cast<DIR*>(lister_),
-                                            &entry,
-                                            &result))) == 0 &&
-      result != NULL) {
+  status = NO_RETRY_EXPECTED(readdir_r(
+      reinterpret_cast<DIR*>(lister_), &entry, &result));
+  if ((status == 0) && (result != NULL)) {
     if (!listing->path_buffer().Add(entry.d_name)) {
       done_ = true;
       return kListError;
     }
     switch (entry.d_type) {
       case DT_DIR:
-        if (strcmp(entry.d_name, ".") == 0) return Next(listing);
-        if (strcmp(entry.d_name, "..") == 0) return Next(listing);
+        if ((strcmp(entry.d_name, ".") == 0) ||
+            (strcmp(entry.d_name, "..") == 0)) {
+          return Next(listing);
+        }
         return kListDirectory;
       case DT_REG:
         return kListFile;
@@ -146,8 +155,8 @@ ListType DirectoryListingEntry::Next(DirectoryListing* listing) {
                                     link_ };
           LinkList* previous = link_;
           while (previous != NULL) {
-            if (previous->dev == current_link.dev &&
-                previous->ino == current_link.ino) {
+            if ((previous->dev == current_link.dev) &&
+                (previous->ino == current_link.ino)) {
               // Report the looping link as a link, rather than following it.
               return kListLink;
             }
@@ -163,14 +172,18 @@ ListType DirectoryListingEntry::Next(DirectoryListing* listing) {
             // Recurse into the subdirectory with current_link added to the
             // linked list of seen file system links.
             link_ = new LinkList(current_link);
-            if (strcmp(entry.d_name, ".") == 0) return Next(listing);
-            if (strcmp(entry.d_name, "..") == 0) return Next(listing);
+            if ((strcmp(entry.d_name, ".") == 0) ||
+                (strcmp(entry.d_name, "..") == 0)) {
+              return Next(listing);
+            }
             return kListDirectory;
           }
         }
         if (S_ISDIR(entry_info.st_mode)) {
-          if (strcmp(entry.d_name, ".") == 0) return Next(listing);
-          if (strcmp(entry.d_name, "..") == 0) return Next(listing);
+          if ((strcmp(entry.d_name, ".") == 0) ||
+              (strcmp(entry.d_name, "..") == 0)) {
+            return Next(listing);
+          }
           return kListDirectory;
         } else if (S_ISREG(entry_info.st_mode)) {
           return kListFile;
@@ -203,7 +216,7 @@ DirectoryListingEntry::~DirectoryListingEntry() {
 
 
 void DirectoryListingEntry::ResetLink() {
-  if (link_ != NULL && (parent_ == NULL || parent_->link_ != link_)) {
+  if ((link_ != NULL) && ((parent_ == NULL) || (parent_->link_ != link_))) {
     delete link_;
     link_ = NULL;
   }
@@ -218,14 +231,15 @@ static bool DeleteRecursively(PathBuffer* path);
 
 static bool DeleteFile(char* file_name,
                        PathBuffer* path) {
-  return path->Add(file_name) && unlink(path->AsString()) == 0;
+  return path->Add(file_name) && (unlink(path->AsString()) == 0);
 }
 
 
 static bool DeleteDir(char* dir_name,
                       PathBuffer* path) {
-  if (strcmp(dir_name, ".") == 0) return true;
-  if (strcmp(dir_name, "..") == 0) return true;
+  if ((strcmp(dir_name, ".") == 0) || (strcmp(dir_name, "..") == 0)) {
+    return true;
+  }
   return path->Add(dir_name) && DeleteRecursively(path);
 }
 
@@ -240,14 +254,16 @@ static bool DeleteRecursively(PathBuffer* path) {
     return (unlink(path->AsString()) == 0);
   }
 
-  if (!path->Add(File::PathSeparator())) return false;
+  if (!path->Add(File::PathSeparator())) {
+    return false;
+  }
 
   // Not a link. Attempt to open as a directory and recurse into the
   // directory.
   DIR* dir_pointer;
   do {
     dir_pointer = opendir(path->AsString());
-  } while (dir_pointer == NULL && errno == EINTR);
+  } while ((dir_pointer == NULL) && (errno == EINTR));
   if (dir_pointer == NULL) {
     return false;
   }
@@ -323,26 +339,26 @@ Directory::ExistsResult Directory::Exists(const char* dir_name) {
       return DOES_NOT_EXIST;
     }
   } else {
-    if (errno == EACCES ||
-        errno == EBADF ||
-        errno == EFAULT ||
-        errno == ENOMEM ||
-        errno == EOVERFLOW) {
+    if ((errno == EACCES) ||
+        (errno == EBADF) ||
+        (errno == EFAULT) ||
+        (errno == ENOMEM) ||
+        (errno == EOVERFLOW)) {
       // Search permissions denied for one of the directories in the
       // path or a low level error occured. We do not know if the
       // directory exists.
       return UNKNOWN;
     }
-    ASSERT(errno == ELOOP ||
-           errno == ENAMETOOLONG ||
-           errno == ENOENT ||
-           errno == ENOTDIR);
+    ASSERT((errno == ELOOP) ||
+           (errno == ENAMETOOLONG) ||
+           (errno == ENOENT) ||
+           (errno == ENOTDIR));
     return DOES_NOT_EXIST;
   }
 }
 
 
-char* Directory::Current() {
+char* Directory::CurrentNoScope() {
   // Android's getcwd adheres closely to the POSIX standard. It won't
   // allocate memory. We need to make our own copy.
 
@@ -355,6 +371,13 @@ char* Directory::Current() {
 }
 
 
+const char* Directory::Current() {
+  char* result = DartUtils::ScopedCString(PATH_MAX);
+  ASSERT(result != NULL);
+  return getcwd(result, PATH_MAX);
+}
+
+
 bool Directory::SetCurrent(const char* path) {
   int result = NO_RETRY_EXPECTED(chdir(path));
   return result == 0;
@@ -366,14 +389,14 @@ bool Directory::Create(const char* dir_name) {
   // process umask.
   int result = NO_RETRY_EXPECTED(mkdir(dir_name, 0777));
   // If the directory already exists, treat it as a success.
-  if (result == -1 && errno == EEXIST) {
+  if ((result == -1) && (errno == EEXIST)) {
     return (Exists(dir_name) == EXISTS);
   }
   return (result == 0);
 }
 
 
-char* Directory::SystemTemp() {
+const char* Directory::SystemTemp() {
   // Android does not have a /tmp directory. A partial substitute,
   // suitable for bring-up work and tests, is to create a tmp
   // directory in /data/local/tmp.
@@ -386,17 +409,19 @@ char* Directory::SystemTemp() {
   if (stat(ANDROID_TEMP_DIR, &st) != 0) {
     mkdir(ANDROID_TEMP_DIR, 0777);
   }
-  return strdup(ANDROID_TEMP_DIR);
+  return ANDROID_TEMP_DIR;
 }
 
 
-char* Directory::CreateTemp(const char* prefix) {
+const char* Directory::CreateTemp(const char* prefix) {
   // Returns a new, unused directory name, adding characters to the end
   // of prefix.  Creates the directory with the permissions specified
   // by the process umask.
-  // The return value must be freed by the caller.
+  // The return value is Dart_ScopeAllocated.
   PathBuffer path;
-  path.Add(prefix);
+  if (!path.Add(prefix)) {
+    return NULL;
+  }
   if (!path.Add("XXXXXX")) {
     // Pattern has overflowed.
     return NULL;
@@ -404,18 +429,18 @@ char* Directory::CreateTemp(const char* prefix) {
   char* result;
   do {
     result = mkdtemp(path.AsString());
-  } while (result == NULL && errno == EINTR);
+  } while ((result == NULL) && (errno == EINTR));
   if (result == NULL) {
     return NULL;
   }
-  return strdup(result);
+  return path.AsScopedString();
 }
 
 
 bool Directory::Delete(const char* dir_name, bool recursive) {
   if (!recursive) {
-    if (File::GetType(dir_name, false) == File::kIsLink &&
-        File::GetType(dir_name, true) == File::kIsDirectory) {
+    if ((File::GetType(dir_name, false) == File::kIsLink) &&
+        (File::GetType(dir_name, true) == File::kIsDirectory)) {
       return (NO_RETRY_EXPECTED(unlink(dir_name)) == 0);
     }
     return (NO_RETRY_EXPECTED(rmdir(dir_name)) == 0);
@@ -431,7 +456,9 @@ bool Directory::Delete(const char* dir_name, bool recursive) {
 
 bool Directory::Rename(const char* path, const char* new_path) {
   ExistsResult exists = Exists(path);
-  if (exists != EXISTS) return false;
+  if (exists != EXISTS) {
+    return false;
+  }
   return (NO_RETRY_EXPECTED(rename(path, new_path)) == 0);
 }