Change how File/Directory/Link .delete works.

runtime/bin/file_win.cc

 // Copyright (c) 2012, the Dart project authors.  Please see the AUTHORS file
 // for details. All rights reserved. Use of this source code is governed by a
 // BSD-style license that can be found in the LICENSE file.
 
 #include "platform/globals.h"
 #if defined(TARGET_OS_WINDOWS)
 
 #include "bin/file.h"
 
 #include <fcntl.h>  // NOLINT
@@ skipping 241 matching lines @@
       NULL);
   if (CloseHandle(dir_handle) == 0) return false;
   free(const_cast<wchar_t*>(target));
   free(reparse_data_buffer);
   return (result != 0);
 }
 
 
 bool File::Delete(const char* name) {
   const wchar_t* system_name = StringUtils::Utf8ToWide(name);
+  int status = _wremove(system_name);
+  free(const_cast<wchar_t*>(system_name));
+  return status != -1;
+}
+
+
+bool File::DeleteLink(const char* name) {
+  const wchar_t* system_name = StringUtils::Utf8ToWide(name);
+  bool result = false;
   DWORD attributes = GetFileAttributesW(system_name);
   if ((attributes != INVALID_FILE_ATTRIBUTES) &&
       (attributes & FILE_ATTRIBUTE_REPARSE_POINT) != 0) {
     // It's a junction(link), delete it.
-    return RemoveDirectoryW(system_name) != 0;
+    result = (RemoveDirectoryW(system_name) != 0);
   } else {
-    int status = _wremove(system_name);
-    free(const_cast<wchar_t*>(system_name));
-    if (status == -1) {
-      return false;
-    }
-    return true;
+    SetLastError(ERROR_NOT_A_REPARSE_POINT);
   }
+  free(const_cast<wchar_t*>(system_name));

[Bill Hesse, 2013/04/08 12:16:33]

I think there is a problem calling "free" between an implicit or explicit
SetLastError, and the GetLastError after the function returns.  See line 333 for
how we wrap a call to free elsewhere.

[Anders Johnsen, 2013/04/08 12:22:34]

Can you link me some info on the problem? I'm not sure what the problem should
be. (it would render any stack-allocated c++ classes dangerous?)

+  return result;
 }
 
 
 off_t File::LengthFromPath(const char* name) {
   struct _stat st;
   const wchar_t* system_name = StringUtils::Utf8ToWide(name);
   int stat_status = _wstat(system_name, &st);
   free(const_cast<wchar_t*>(system_name));
   if (stat_status == 0) {
     return st.st_size;
@@ skipping 188 matching lines @@
 
 File::StdioHandleType File::GetStdioHandleType(int fd) {
   // Treat all stdio handles as pipes. The Windows event handler and
   // socket code will handle the different handle types.
   return kPipe;
 }
 
 
 File::Type File::GetType(const char* pathname, bool follow_links) {
   const wchar_t* name = StringUtils::Utf8ToWide(pathname);
-  WIN32_FIND_DATAW file_data;
-  HANDLE find_handle = FindFirstFileW(name, &file_data);
-  if (find_handle == INVALID_HANDLE_VALUE) {
-    // TODO(whesse): Distinguish other errors from does not exist.
-    return File::kDoesNotExist;
-  }
-  FindClose(find_handle);
-  DWORD attributes = file_data.dwFileAttributes;
-  if ((attributes & FILE_ATTRIBUTE_REPARSE_POINT) != 0) {
+  DWORD attributes = GetFileAttributesW(name);
+  File::Type result = kIsFile;
+  if (attributes == INVALID_FILE_ATTRIBUTES) {
+    result = kDoesNotExist;
+  } else if ((attributes & FILE_ATTRIBUTE_REPARSE_POINT) != 0) {
     if (follow_links) {
       HANDLE dir_handle = CreateFileW(
           name,
           0,
           FILE_SHARE_READ | FILE_SHARE_WRITE | FILE_SHARE_DELETE,
           NULL,
           OPEN_EXISTING,
           FILE_FLAG_BACKUP_SEMANTICS,
           NULL);
       if (dir_handle == INVALID_HANDLE_VALUE) {
-        // TODO(whesse): Distinguish other errors from does not exist.
-        return File::kDoesNotExist;
+        result = File::kIsLink;
       } else {
         CloseHandle(dir_handle);

[Bill Hesse, 2013/04/08 12:16:33]

Since we have the handle to the file, and this might be a symbolic link, rather
than a junction, should we check the attributes for file or directory (or even
the original "attributes" field), and report file or directory here?  Since we
don't support creating symbolic links, we couldn't test this.

[Anders Johnsen, 2013/04/08 12:22:34]

I understand now. Yes, we should probably add that back.

-        return File::kIsDirectory;
+        result = File::kIsDirectory;
       }
     } else {
-      DWORD reparse_tag = file_data.dwReserved0;
-      if (reparse_tag == IO_REPARSE_TAG_SYMLINK ||
-          reparse_tag == IO_REPARSE_TAG_MOUNT_POINT) {
-        return File::kIsLink;
-      } else {
-        return File::kDoesNotExist;
-      }
+      result = kIsLink;
     }
   } else if ((attributes & FILE_ATTRIBUTE_DIRECTORY) != 0) {
-    return File::kIsDirectory;
-  } else {
-    return File::kIsFile;
+    result = kIsDirectory;
   }
+  free(const_cast<wchar_t*>(name));
+  return result;
 }
 
 
 File::Identical File::AreIdentical(const char* file_1, const char* file_2) {
   BY_HANDLE_FILE_INFORMATION file_info[2];
   const char* file_names[2] = { file_1, file_2 };
   for (int i = 0; i < 2; ++i) {
     const wchar_t* wide_name = StringUtils::Utf8ToWide(file_names[i]);
     HANDLE file_handle = CreateFileW(
         wide_name,
@@ skipping 24 matching lines @@
   if (file_info[0].dwVolumeSerialNumber == file_info[1].dwVolumeSerialNumber &&
       file_info[0].nFileIndexHigh == file_info[1].nFileIndexHigh &&
       file_info[0].nFileIndexLow == file_info[1].nFileIndexLow) {
     return kIdentical;
   } else {
     return kDifferent;
   }
 }
 
 #endif  // defined(TARGET_OS_WINDOWS)