dart:io: Use PathBuffer class for Directory.list on Windows platform.

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 9d8f6f31935d30691c4344acc8354d300dfef835..84242c622fae992cd754fa4e5cebf90015b488d4 100644
--- a/runtime/bin/directory_win.cc
+++ b/runtime/bin/directory_win.cc
@@ -9,51 +9,75 @@
 
 #include "bin/log.h"
 
-// Forward declaration.
+class PathBuffer {
+ public:
+  PathBuffer() : length(0) { }
+
+  wchar_t data[MAX_PATH + 1];
+  int length;
+
+  bool Add(const wchar_t* name) {
+    size_t written = _snwprintf(data + length,
+                               MAX_PATH - length,
+                               L"%s",
+                               name);
+    data[MAX_PATH] = L'\0';
+    if (written == wcsnlen(name, MAX_PATH + 1)) {
+      length += written;
+      return true;
+    } else {
+      SetLastError(ERROR_BUFFER_OVERFLOW);
+      return false;
+    }
+  }
+
+  void Reset(int new_length) {
+    length = new_length;
+    data[length] = L'\0';
+  }
+};
+
+
+// Forward declarations.
 static bool ListRecursively(const wchar_t* dir_name,
                             bool recursive,
                             DirectoryListing* listing);
 static bool DeleteRecursively(const wchar_t* dir_name);
 
 
+static void PostError(DirectoryListing* listing,
+                      const wchar_t* dir_name) {
+  const char* utf8_path = StringUtils::WideToUtf8(dir_name);
+  listing->HandleError(utf8_path);
+  free(const_cast<char*>(utf8_path));
+}
+
+
 static bool HandleDir(wchar_t* dir_name,
-                      wchar_t* path,
-                      int path_length,
+                      PathBuffer* path,
                       bool recursive,
                       DirectoryListing* listing) {
-  if (wcscmp(dir_name, L".") != 0 &&
-      wcscmp(dir_name, L"..") != 0) {
-    size_t written = _snwprintf(path + path_length,
-                                MAX_PATH - path_length,
-                                L"%s",
-                                dir_name);
-    if (written != wcslen(dir_name)) {
-      return false;
-    }
-    char* utf8_path = StringUtils::WideToUtf8(path);
-    bool ok = listing->HandleDirectory(utf8_path);
-    free(utf8_path);
-    if (!ok) return ok;
-    if (recursive) {
-      return ListRecursively(path, recursive, listing);
-    }
+  if (wcscmp(dir_name, L".") == 0) return true;
+  if (wcscmp(dir_name, L"..") == 0) return true;
+  if (!path->Add(dir_name)) {
+    PostError(listing, path->data);
+    return false;
   }
-  return true;
+  char* utf8_path = StringUtils::WideToUtf8(path->data);
+  bool ok = listing->HandleDirectory(utf8_path);
+  free(utf8_path);
+  return ok && (!recursive || ListRecursively(path->data, recursive, listing));

[Bill Hesse, 2013/02/13 14:41:40]

Line 69 is the only change from the previously committed version of this change.
 The previous version fell off the end and returned null if recursive was false,
because the return true from the current code was removed.

 }
 
 
 static bool HandleFile(wchar_t* file_name,
-                       wchar_t* path,
-                       int path_length,
+                       PathBuffer* path,
                        DirectoryListing* listing) {
-  size_t written = _snwprintf(path + path_length,
-                              MAX_PATH - path_length,
-                              L"%s",
-                              file_name);
-  if (written != wcslen(file_name)) {
+  if (!path->Add(file_name)) {
+    PostError(listing, path->data);
     return false;
-  };
-  char* utf8_path = StringUtils::WideToUtf8(path);
+  }
+  char* utf8_path = StringUtils::WideToUtf8(path->data);
   bool ok = listing->HandleFile(utf8_path);
   free(utf8_path);
   return ok;
@@ -61,55 +85,42 @@ static bool HandleFile(wchar_t* file_name,
 
 
 static bool HandleEntry(LPWIN32_FIND_DATAW find_file_data,
-                        wchar_t* path,
-                        int path_length,
+                        PathBuffer* path,
                         bool recursive,
                         DirectoryListing* listing) {
   DWORD attributes = find_file_data->dwFileAttributes;
   if ((attributes & FILE_ATTRIBUTE_DIRECTORY) != 0) {
     return HandleDir(find_file_data->cFileName,
                      path,
-                     path_length,
                      recursive,
                      listing);
   } else {
-    return HandleFile(find_file_data->cFileName, path, path_length, listing);
+    return HandleFile(find_file_data->cFileName, path, listing);
   }
 }
 
 
-// ComputeFullSearchPath must be called with a path array of size at
-// least MAX_PATH.
-static bool ComputeFullSearchPath(const wchar_t* dir_name,
-                                  wchar_t* path,
-                                  int* path_length) {
+static PathBuffer* ComputeFullSearchPath(const wchar_t* dir_name) {
   // GetFullPathName only works in a multi-threaded environment if
   // SetCurrentDirectory is not used. We currently have no plan for
   // exposing SetCurrentDirectory.
-  size_t written = GetFullPathNameW(dir_name, MAX_PATH, path, NULL);
+  PathBuffer* path = new PathBuffer();
+
+  size_t written = GetFullPathNameW(dir_name, MAX_PATH + 1, path->data, NULL);
   // GetFullPathName only accepts input strings of size less than
   // MAX_PATH and returns 0 to indicate failure for paths longer than
   // that. Therefore the path buffer is always big enough.
   if (written == 0 || written > MAX_PATH) {
-    return false;
+    delete path;
+    return NULL;
   }
-  *path_length = written;
-  written = _snwprintf(path + *path_length,
-                       MAX_PATH - *path_length,
-                       L"%s",
-                       L"\\*");
-  if (written != 2) {
-    return false;
+  path->length = written;
+  if (path->Add(L"\\*")) {
+    return path;
+  } else {
+    delete path;
+    return NULL;
   }
-  *path_length += written;
-  return true;
-}
-
-static void PostError(DirectoryListing* listing,
-                      const wchar_t* dir_name) {
-  const char* utf8_path = StringUtils::WideToUtf8(dir_name);
-  listing->HandleError(utf8_path);
-  free(const_cast<char*>(utf8_path));
 }
 
 
@@ -121,38 +132,35 @@ static bool ListRecursively(const wchar_t* dir_name,
   // recursive traversal. path_length does not always equal
   // strlen(path) but indicates the current prefix of path that is the
   // path of the current directory in the traversal.
-  wchar_t* path = static_cast<wchar_t*>(malloc(MAX_PATH * sizeof(wchar_t)));
-  int path_length = 0;
-  bool valid = ComputeFullSearchPath(dir_name, path, &path_length);
-  if (!valid) {
+  PathBuffer* path = ComputeFullSearchPath(dir_name);
+  if (path == NULL) {
     PostError(listing, dir_name);
-    free(path);
+    delete path;
     return false;
   }
 
   WIN32_FIND_DATAW find_file_data;
-  HANDLE find_handle = FindFirstFileW(path, &find_file_data);
+  HANDLE find_handle = FindFirstFileW(path->data, &find_file_data);
 
   // Adjust the path by removing the '*' used for the search.
-  path_length -= 1;
-  path[path_length] = '\0';
+  path->Reset(path->length - 1);
 
   if (find_handle == INVALID_HANDLE_VALUE) {
-    PostError(listing, path);
-    free(path);
+    PostError(listing, path->data);
+    delete path;
     return false;
   }
 
+  int path_length = path->length;
   bool success = HandleEntry(&find_file_data,
                              path,
-                             path_length,
                              recursive,
                              listing);
 
   while ((FindNextFileW(find_handle, &find_file_data) != 0)) {
+    path->Reset(path_length);  // HandleEntry adds the entry name to path.
     success = HandleEntry(&find_file_data,
                           path,
-                          path_length,
                           recursive,
                           listing) && success;
   }
@@ -166,24 +174,16 @@ static bool ListRecursively(const wchar_t* dir_name,
     success = false;
     PostError(listing, dir_name);
   }
-  free(path);
+  delete path;
 
   return success;
 }
 
 
-static bool DeleteFile(wchar_t* file_name,
-                       wchar_t* path,
-                       int path_length) {
-  size_t written = _snwprintf(path + path_length,
-                              MAX_PATH - path_length,
-                              L"%s",
-                              file_name);
-  if (written != wcslen(file_name)) {
-    return false;
-  }
+static bool DeleteFile(wchar_t* file_name, PathBuffer* path) {
+  if (!path->Add(file_name)) return false;
 
-  if (DeleteFileW(path) != 0) {
+  if (DeleteFileW(path->data) != 0) {
     return true;
   }
 
@@ -191,7 +191,7 @@ static bool DeleteFile(wchar_t* file_name,
   // again. This mirrors Linux/Mac where a directory containing read-only files
   // can still be recursively deleted.
   if (GetLastError() == ERROR_ACCESS_DENIED) {
-    DWORD attributes = GetFileAttributesW(path);
+    DWORD attributes = GetFileAttributesW(path->data);
     if (attributes == INVALID_FILE_ATTRIBUTES) {
       return false;
     }
@@ -199,11 +199,11 @@ static bool DeleteFile(wchar_t* file_name,
     if ((attributes & FILE_ATTRIBUTE_READONLY) == FILE_ATTRIBUTE_READONLY) {
       attributes &= ~FILE_ATTRIBUTE_READONLY;
 
-      if (SetFileAttributesW(path, attributes) == 0) {
+      if (SetFileAttributesW(path->data, attributes) == 0) {
         return false;
       }
 
-      return DeleteFileW(path) != 0;
+      return DeleteFileW(path->data) != 0;
     }
   }
 
@@ -211,33 +211,20 @@ static bool DeleteFile(wchar_t* file_name,
 }
 
 
-static bool DeleteDir(wchar_t* dir_name,
-                      wchar_t* path,
-                      int path_length) {
-  if (wcscmp(dir_name, L".") != 0 &&
-      wcscmp(dir_name, L"..") != 0) {
-    size_t written = _snwprintf(path + path_length,
-                                MAX_PATH - path_length,
-                                L"%s",
-                                dir_name);
-    if (written != wcslen(dir_name)) {
-      return false;
-    }
-    return DeleteRecursively(path);
-  }
-  return true;
+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;
+  return path->Add(dir_name) && DeleteRecursively(path->data);
 }
 
 
-static bool DeleteEntry(LPWIN32_FIND_DATAW find_file_data,
-                        wchar_t* path,
-                        int path_length) {
+static bool DeleteEntry(LPWIN32_FIND_DATAW find_file_data, PathBuffer* path) {
   DWORD attributes = find_file_data->dwFileAttributes;
 
   if ((attributes & FILE_ATTRIBUTE_DIRECTORY) != 0) {
-    return DeleteDir(find_file_data->cFileName, path, path_length);
+    return DeleteDir(find_file_data->cFileName, path);
   } else {
-    return DeleteFile(find_file_data->cFileName, path, path_length);
+    return DeleteFile(find_file_data->cFileName, path);
   }
 }
 
@@ -257,33 +244,29 @@ static bool DeleteRecursively(const wchar_t* dir_name) {
   // recursive traversal. path_length does not always equal
   // strlen(path) but indicates the current prefix of path that is the
   // path of the current directory in the traversal.
-  wchar_t* path = static_cast<wchar_t*>(malloc(MAX_PATH * sizeof(wchar_t)));
-  int path_length = 0;
-  bool valid = ComputeFullSearchPath(dir_name, path, &path_length);
-  if (!valid) {
-    free(path);
-    return false;
-  }
+  PathBuffer* path = ComputeFullSearchPath(dir_name);
+  if (path == NULL) return false;
 
   WIN32_FIND_DATAW find_file_data;
-  HANDLE find_handle = FindFirstFileW(path, &find_file_data);
+  HANDLE find_handle = FindFirstFileW(path->data, &find_file_data);
 
   // Adjust the path by removing the '*' used for the search.
-  path_length -= 1;
-  path[path_length] = '\0';
+  int path_length = path->length - 1;
+  path->Reset(path_length);
 
   if (find_handle == INVALID_HANDLE_VALUE) {
-    free(path);
+    delete path;
     return false;
   }
 
-  bool success = DeleteEntry(&find_file_data, path, path_length);
+  bool success = DeleteEntry(&find_file_data, path);
 
   while ((FindNextFileW(find_handle, &find_file_data) != 0) && success) {
-    success = success && DeleteEntry(&find_file_data, path, path_length);
+    path->Reset(path_length);  // DeleteEntry adds to the path.
+    success = success && DeleteEntry(&find_file_data, path);
   }
 
-  free(path);
+  delete path;
 
   if ((GetLastError() != ERROR_NO_MORE_FILES) ||
       (FindClose(find_handle) == 0) ||
@@ -362,52 +345,51 @@ char* Directory::CreateTemp(const char* const_template) {
   // dir_template.  Creates this directory, with a default security
   // descriptor inherited from its parent directory.
   // The return value must be freed by the caller.
-  wchar_t* path = static_cast<wchar_t*>(malloc(MAX_PATH * sizeof(wchar_t)));
-  int path_length;
+  PathBuffer* path = new PathBuffer();
   if (0 == strncmp(const_template, "", 1)) {
-    path_length = GetTempPathW(MAX_PATH, path);
-    if (path_length == 0) {
-      free(path);
+    path->length = GetTempPathW(MAX_PATH, path->data);
+    if (path->length == 0) {
+      delete path;
       return NULL;
     }
   } else {
     const wchar_t* system_template = StringUtils::Utf8ToWide(const_template);
-    _snwprintf(path, MAX_PATH, L"%s", system_template);
+    path->Add(system_template);
     free(const_cast<wchar_t*>(system_template));
-    path_length = wcslen(path);
   }
   // Length of tempdir-xxxxxxxx-xxxx-xxxx-xxxx-xxxxxxxxxxxx is 44.
-  if (path_length > MAX_PATH - 44) {
-    free(path);
+  if (path->length > MAX_PATH - 44) {
+    delete path;
     return NULL;
   }
-  if ((path)[path_length - 1] == L'\\') {
+  if ((path->data)[path->length - 1] == L'\\') {
     // No base name for the directory - use "tempdir".
-    _snwprintf(path + path_length, MAX_PATH - path_length, L"tempdir");
-    path_length = wcslen(path);
+    path->Add(L"tempdir");
   }
 
   UUID uuid;
   RPC_STATUS status = UuidCreateSequential(&uuid);
   if (status != RPC_S_OK && status != RPC_S_UUID_LOCAL_ONLY) {
-    free(path);
+    delete path;
     return NULL;
   }
   RPC_WSTR uuid_string;
   status = UuidToStringW(&uuid, &uuid_string);
   if (status != RPC_S_OK) {
-    free(path);
+    delete path;
     return NULL;
   }
 
-  _snwprintf(path + path_length, MAX_PATH - path_length, L"-%s", uuid_string);
+  path->Add(L"-");
+  // RPC_WSTR is an unsigned short*, so we cast to wchar_t*.
+  path->Add(reinterpret_cast<wchar_t*>(uuid_string));
   RpcStringFreeW(&uuid_string);
-  if (!CreateDirectoryW(path, NULL)) {
-    free(path);
+  if (!CreateDirectoryW(path->data, NULL)) {
+    delete path;
     return NULL;
   }
-  char* result = StringUtils::WideToUtf8(path);
-  free(path);
+  char* result = StringUtils::WideToUtf8(path->data);
+  delete path;
   return result;
 }