Fixes some memory leaks in //runtime/bin

runtime/bin/directory_win.cc

Index: runtime/bin/directory_win.cc
diff --git a/runtime/bin/directory_win.cc b/runtime/bin/directory_win.cc
index 40bfb9bf158ddf36b53217204bf56c9589a373c1..6fc2ae9f5fadfe47ce417e70d6938db4397b0192 100644
--- a/runtime/bin/directory_win.cc
+++ b/runtime/bin/directory_win.cc
@@ -13,6 +13,7 @@
 #include <errno.h>  // NOLINT
 #include <sys/stat.h>  // NOLINT
 
+#include "bin/dartutils.h"
 #include "bin/log.h"
 
 #undef DeleteFile
@@ -23,24 +24,32 @@ namespace dart {
 namespace bin {
 
 PathBuffer::PathBuffer() : length_(0) {
-  data_ = calloc(MAX_LONG_PATH + 1,  sizeof(wchar_t));  // NOLINT
+  data_ = calloc(MAX_LONG_PATH + 1, sizeof(wchar_t));  // NOLINT
 }
 
+
 char* PathBuffer::AsString() const {
-  return StringUtilsWin::WideToUtf8(AsStringW());
+  UNREACHABLE();
+  return NULL;
 }
 
+
 wchar_t* PathBuffer::AsStringW() const {
   return reinterpret_cast<wchar_t*>(data_);
 }
 
+
+const char* PathBuffer::AsScopedString() const {
+  return StringUtilsWin::WideToUtf8(AsStringW());
+}
+
+
 bool PathBuffer::Add(const char* name) {
   const wchar_t* wide_name = StringUtilsWin::Utf8ToWide(name);
-  bool success = AddW(wide_name);
-  free(const_cast<wchar_t*>(wide_name));
-  return success;
+  return AddW(wide_name);
 }
 
+
 bool PathBuffer::AddW(const wchar_t* name) {
   wchar_t* data = AsStringW();
   int written = _snwprintf(data + length_,
@@ -48,9 +57,9 @@ bool PathBuffer::AddW(const wchar_t* name) {
                            L"%s",
                            name);
   data[MAX_LONG_PATH] = L'\0';
-  if (written <= MAX_LONG_PATH - length_ &&
-      written >= 0 &&
-      static_cast<size_t>(written) == wcsnlen(name, MAX_LONG_PATH + 1)) {
+  if ((written <= MAX_LONG_PATH - length_) &&
+      (written >= 0) &&
+      (static_cast<size_t>(written) == wcsnlen(name, MAX_LONG_PATH + 1))) {
     length_ += written;
     return true;
   } else {
@@ -59,11 +68,13 @@ bool PathBuffer::AddW(const wchar_t* name) {
   }
 }
 
-void PathBuffer::Reset(int new_length) {
+
+void PathBuffer::Reset(intptr_t new_length) {
   length_ = new_length;
   AsStringW()[length_] = L'\0';
 }
 
+
 // If link_name points to a link, IsBrokenLink will return true if link_name
 // points to an invalid target.
 static bool IsBrokenLink(const wchar_t* link_name) {
@@ -83,6 +94,7 @@ static bool IsBrokenLink(const wchar_t* link_name) {
   }
 }
 
+
 // A linked list structure holding a link target's unique file system ID.
 // Used to detect loops in the file system when listing recursively.
 struct LinkList {
@@ -98,7 +110,7 @@ static bool DeleteRecursively(PathBuffer* path);
 
 static ListType HandleFindFile(DirectoryListing* listing,
                                DirectoryListingEntry* entry,
-                               WIN32_FIND_DATAW& find_file_data) {
+                               const WIN32_FIND_DATAW& find_file_data) {
   if (!listing->path_buffer().AddW(find_file_data.cFileName)) {
     return kListError;
   }
@@ -137,17 +149,17 @@ static ListType HandleFindFile(DirectoryListing* listing,
       current_link.next = entry->link();
       LinkList* previous = entry->link();
       while (previous != NULL) {
-        if (previous->volume == current_link.volume &&
-            previous->id_low == current_link.id_low &&
-            previous->id_high == current_link.id_high) {
+        if ((previous->volume == current_link.volume) &&
+            (previous->id_low == current_link.id_low) &&
+            (previous->id_high == current_link.id_high)) {
           // Report the looping link as a link, rather than following it.
           return kListLink;
         }
         previous = previous->next;
       }
       // Recurse into the directory, adding current link to the seen links list.
-      if (wcscmp(find_file_data.cFileName, L".") == 0 ||
-          wcscmp(find_file_data.cFileName, L"..") == 0) {
+      if ((wcscmp(find_file_data.cFileName, L".") == 0) ||
+          (wcscmp(find_file_data.cFileName, L"..") == 0)) {
         return entry->Next(listing);
       }
       entry->set_link(new LinkList(current_link));
@@ -155,8 +167,8 @@ static ListType HandleFindFile(DirectoryListing* listing,
     }
   }
   if ((attributes & FILE_ATTRIBUTE_DIRECTORY) != 0) {
-    if (wcscmp(find_file_data.cFileName, L".") == 0 ||
-        wcscmp(find_file_data.cFileName, L"..") == 0) {
+    if ((wcscmp(find_file_data.cFileName, L".") == 0) ||
+        (wcscmp(find_file_data.cFileName, L"..") == 0)) {
       return entry->Next(listing);
     }
     return kListDirectory;
@@ -165,6 +177,7 @@ static ListType HandleFindFile(DirectoryListing* listing,
   }
 }
 
+
 ListType DirectoryListingEntry::Next(DirectoryListing* listing) {
   if (done_) {
     return kListDone;
@@ -223,7 +236,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;
   }
@@ -234,7 +247,9 @@ void DirectoryListingEntry::ResetLink() {
 
 
 static bool DeleteFile(wchar_t* file_name, PathBuffer* path) {
-  if (!path->AddW(file_name)) return false;
+  if (!path->AddW(file_name)) {
+    return false;
+  }
 
   if (DeleteFileW(path->AsStringW()) != 0) {
     return true;
@@ -265,8 +280,10 @@ static bool DeleteFile(wchar_t* file_name, PathBuffer* path) {
 
 
 static bool DeleteDir(wchar_t* dir_name, PathBuffer* path) {
-  if (wcscmp(dir_name, L".") == 0) return true;
-  if (wcscmp(dir_name, L"..") == 0) return true;
+  if ((wcscmp(dir_name, L".") == 0) ||
+      (wcscmp(dir_name, L"..") == 0)) {
+    return true;
+  }
   return path->AddW(dir_name) && DeleteRecursively(path);
 }
 
@@ -284,7 +301,7 @@ static bool DeleteEntry(LPWIN32_FIND_DATAW find_file_data, PathBuffer* path) {
 
 static bool DeleteRecursively(PathBuffer* path) {
   DWORD attributes = GetFileAttributesW(path->AsStringW());
-  if ((attributes == INVALID_FILE_ATTRIBUTES)) {
+  if (attributes == INVALID_FILE_ATTRIBUTES) {
     return false;
   }
   // If the directory is a junction, it's pointing to some other place in the
@@ -298,7 +315,9 @@ static bool DeleteRecursively(PathBuffer* path) {
     return DeleteFile(L"", path);
   }
 
-  if (!path->AddW(L"\\*")) return false;
+  if (!path->AddW(L"\\*")) {
+    return false;
+  }
 
   WIN32_FIND_DATAW find_file_data;
   HANDLE find_handle = FindFirstFileW(path->AsStringW(), &find_file_data);
@@ -336,8 +355,8 @@ static Directory::ExistsResult ExistsHelper(const wchar_t* dir_name) {
   DWORD attributes = GetFileAttributesW(dir_name);
   if (attributes == INVALID_FILE_ATTRIBUTES) {
     DWORD last_error = GetLastError();
-    if (last_error == ERROR_FILE_NOT_FOUND ||
-        last_error == ERROR_PATH_NOT_FOUND) {
+    if ((last_error == ERROR_FILE_NOT_FOUND) ||
+        (last_error == ERROR_PATH_NOT_FOUND)) {
       return Directory::DOES_NOT_EXIST;
     } else {
       // We might not be able to get the file attributes for other
@@ -354,27 +373,42 @@ static Directory::ExistsResult ExistsHelper(const wchar_t* dir_name) {
 
 Directory::ExistsResult Directory::Exists(const char* dir_name) {
   const wchar_t* system_name = StringUtilsWin::Utf8ToWide(dir_name);
-  Directory::ExistsResult result = ExistsHelper(system_name);
-  free(const_cast<wchar_t*>(system_name));
-  return result;
+  return ExistsHelper(system_name);
 }
 
 
-char* Directory::Current() {
+char* Directory::CurrentNoScope() {
   int length = GetCurrentDirectoryW(0, NULL);
-  if (length == 0) return NULL;
+  if (length == 0) {
+    return NULL;
+  }
   wchar_t* current = new wchar_t[length + 1];
   GetCurrentDirectoryW(length + 1, current);
-  char* result = StringUtilsWin::WideToUtf8(current);
+  int utf8_len = WideCharToMultiByte(
+      CP_UTF8, 0, current, -1, NULL, 0, NULL, NULL);
+  char* result = reinterpret_cast<char*>(malloc(utf8_len));
+  WideCharToMultiByte(CP_UTF8, 0, current, -1, result, utf8_len, NULL, NULL);
   delete[] current;
   return result;
 }
 
 
+const char* Directory::Current() {
+  int length = GetCurrentDirectoryW(0, NULL);
+  if (length == 0) {
+    return NULL;
+  }
+  wchar_t* current;
+  current = reinterpret_cast<wchar_t*>(
+      Dart_ScopeAllocate((length + 1) * sizeof(*current)));
+  GetCurrentDirectoryW(length + 1, current);
+  return StringUtilsWin::WideToUtf8(current);
+}
+
+
 bool Directory::SetCurrent(const char* path) {
   const wchar_t* system_path = StringUtilsWin::Utf8ToWide(path);
   bool result = SetCurrentDirectoryW(system_path) != 0;
-  free(const_cast<wchar_t*>(system_path));
   return result;
 }
 
@@ -383,35 +417,34 @@ bool Directory::Create(const char* dir_name) {
   const wchar_t* system_name = StringUtilsWin::Utf8ToWide(dir_name);
   int create_status = CreateDirectoryW(system_name, NULL);
   // If the directory already existed, treat it as a success.
-  if (create_status == 0 &&
-      GetLastError() == ERROR_ALREADY_EXISTS &&
-      ExistsHelper(system_name) == EXISTS) {
-    free(const_cast<wchar_t*>(system_name));
+  if ((create_status == 0) &&
+      (GetLastError() == ERROR_ALREADY_EXISTS) &&
+      (ExistsHelper(system_name) == EXISTS)) {
     return true;
   }
-  free(const_cast<wchar_t*>(system_name));
   return (create_status != 0);
 }
 
 
-char* Directory::SystemTemp() {
+const char* Directory::SystemTemp() {
   PathBuffer path;
   // Remove \ at end.
   path.Reset(GetTempPathW(MAX_LONG_PATH, path.AsStringW()) - 1);
-  return path.AsString();
+  return path.AsScopedString();
 }
 
 
-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 this directory, with a default security
   // descriptor inherited from its parent directory.
-  // The return value must be freed by the caller.
+  // The return value is Dart_ScopeAllocated.
   PathBuffer path;
   const wchar_t* system_prefix = StringUtilsWin::Utf8ToWide(prefix);
-  path.AddW(system_prefix);
-  free(const_cast<wchar_t*>(system_prefix));
+  if (!path.AddW(system_prefix)) {
+    return NULL;
+  }
 
   // Length of xxxxxxxx-xxxx-xxxx-xxxx-xxxxxxxxxxxx is 36.
   if (path.length() > MAX_LONG_PATH - 36) {
@@ -420,7 +453,7 @@ char* Directory::CreateTemp(const char* prefix) {
 
   UUID uuid;
   RPC_STATUS status = UuidCreateSequential(&uuid);
-  if (status != RPC_S_OK && status != RPC_S_UUID_LOCAL_ONLY) {
+  if ((status != RPC_S_OK) && (status != RPC_S_UUID_LOCAL_ONLY)) {
     return NULL;
   }
   RPC_WSTR uuid_string;
@@ -430,13 +463,14 @@ char* Directory::CreateTemp(const char* prefix) {
   }
 
   // RPC_WSTR is an unsigned short*, so we cast to wchar_t*.
-  path.AddW(reinterpret_cast<wchar_t*>(uuid_string));
+  if (!path.AddW(reinterpret_cast<wchar_t*>(uuid_string))) {
+    return NULL;
+  }
   RpcStringFreeW(&uuid_string);
   if (!CreateDirectoryW(path.AsStringW(), NULL)) {
     return NULL;
   }
-  char* result = path.AsString();
-  return result;
+  return path.AsScopedString();
 }
 
 
@@ -455,7 +489,6 @@ bool Directory::Delete(const char* dir_name, bool recursive) {
       result = DeleteRecursively(&path);
     }
   }
-  free(const_cast<wchar_t*>(system_dir_name));
   return result;
 }
 
@@ -464,20 +497,22 @@ bool Directory::Rename(const char* path, const char* new_path) {
   const wchar_t* system_path = StringUtilsWin::Utf8ToWide(path);
   const wchar_t* system_new_path = StringUtilsWin::Utf8ToWide(new_path);
   ExistsResult exists = ExistsHelper(system_path);
-  if (exists != EXISTS) return false;

[zra, 2016/03/11 23:55:50]

leaked system_path and system_new_path.

+  if (exists != EXISTS) {
+    return false;
+  }
   ExistsResult new_exists = ExistsHelper(system_new_path);
   // MoveFile does not allow replacing exising directories. Therefore,
   // if the new_path is currently a directory we need to delete it
   // first.
   if (new_exists == EXISTS) {
     bool success = Delete(new_path, true);
-    if (!success) return false;

[zra, 2016/03/11 23:55:50]

ditto

+    if (!success) {
+      return false;
+    }
   }
   DWORD flags = MOVEFILE_WRITE_THROUGH;
   int move_status =
       MoveFileExW(system_path, system_new_path, flags);
-  free(const_cast<wchar_t*>(system_path));
-  free(const_cast<wchar_t*>(system_new_path));
   return (move_status != 0);
 }