[11/N] [libcxx] Hook up a number of operation functions to their windows counterparts

[mstorsjo]

Summary

Use the corresponding wchar functions, named "_wfunc" instead of "func", where feasible, or reimplement functions with native windows APIs.

[amccarth]

I have a couple questions to consider inline. If those have already been thought through, then LTGM.

libcxx/src/filesystem/operations.cpp

518

Technically, there's nothing wrong here. It just _feels_ a little sketchy to call GetLastError() twice in a row.

539

Why not implement this with ::DeleteFile()? Does it have to do with waiting until the last handle is closed?

561

I'm mildly concerned about the way files are sometimes manipulated by HANDLE and sometimes by file descriptor.

If fd represents a file in text mode, then the caller needs to be very careful to understand that length must be the _byte_ offset, not the _char_ offset into the file.

573

Consider also MOVEFILE_WRITE_THROUGH for the case that it is indeed copied and then (probably) deleted.

[mstorsjo]

libcxx/src/filesystem/operations.cpp

518

Yeah, it's not ideal, but then again, making a separate version of the set_errno() function that takes an error code as input, just for this case, also feels a bit superfluous (as the first call to GetLastError() shouldn't clobber the first one). Or would a default parameter, like set_errno(int err = GetLastError()) work?

539

Haven't considered/tested that, I can try that and run it over the testcases to see if it works.

The reason I ended up with this, was that I first just called _wremove(), followed by a _wrmdir() if the first failed and the path is a directory - as that was the most obvious path forward when porting over from the posix remove() function. That worked mostly, but failed in some symlink cases, and then I peeked at how the MS STL (which has the same license as libcxx, fwiw) implemented it, and I did the same.

561

File descriptors here are only an internal detail, fwiw, they're only used within the __copy_file function. But good catch about text vs binary mode; I guess the calls to FileDescriptor::create_with_status need to be amended with O_BINARY.

573

Sounds like a good thing to have, I can add that. These flags were based on what the MS STL use, and I didn't see that flag used there though.

[mstorsjo]

libcxx/src/filesystem/operations.cpp

518

Changed set_errno() to take the error code as a parameter, with the parameter set by default to GetLastError().

539

I tried with this implementation, but this still fails on removing symlinks to directories:

int remove(const wchar_t *path) {
  if (DeleteFileW(path))
    return 0;
  int e = GetLastError();
  if (e == ERROR_ACCESS_DENIED) {
    if (is_directory(path)) {
      if (!RemoveDirectoryW(path)) 
        return set_errno();
      return 0;
    }
  }
  return set_errno(e);
}

561

Adding O_BINARY at the callsites that open file descriptors.

573

Adding that flag as it sounds beneficial, despite MS STL not using it.

[mstorsjo]

Made symlink_file_dir pass the inspected error code to set_errno(), passing O_BINARY to the open() function, passing the MOVEFILE_WRITE_THROUGH flag to MoveFileExW.

[amccarth]

LGTM, now.

[mstorsjo]

Rebased, already formally approved.

[mstorsjo]

Rebased, moved helpers to posix_compat.h.

[mstorsjo]

Retrigger CI.

[ldionne]

The integration between libc++ and the Windows shim looks great. I didn't look at the actual implementations using Windows APIs (I don't know those APIs and I trust @amccarth with that). LGTM once CI passes (can you please rebase to trigger, I'll add the LLVM monorepo to the diff now).

[mstorsjo]

Rebased, rerunning CI before pushing

libcxx/src/filesystem/filesystem_common.h

 #define S_ISDIR(m)      (((m) & _S_IFMT) == _S_IFDIR)
 #define S_ISCHR(m)      (((m) & _S_IFMT) == _S_IFCHR)
 #define S_ISFIFO(m)     (((m) & _S_IFMT) == _S_IFIFO)
 #define S_ISREG(m)      (((m) & _S_IFMT) == _S_IFREG)
 #define S_ISBLK(m)      (((m) & _S_IFMT) == _S_IFBLK)
 #define S_ISLNK(m)      (((m) & _S_IFMT) == _S_IFLNK)
 #define S_ISSOCK(m)     (((m) & _S_IFMT) == _S_IFSOCK)
 
+#define O_NONBLOCK 0
+
 #else
 using TimeSpec = timespec;
 using StatT = struct stat;
+
+#define O_BINARY 0
+
 #endif
 
 template <class FileTimeT, class TimeT,
           bool IsFloat = is_floating_point<typename FileTimeT::rep>::value>
 struct time_util_base {
   using rep = typename FileTimeT::rep;
   using fs_duration = typename FileTimeT::duration;
   using fs_seconds = duration<rep>;

libcxx/src/filesystem/operations.cpp

int lstat(const wchar_t *path, StatT *buf) {
   return stat_file(path, buf, FILE_FLAG_OPEN_REPARSE_POINT);
 }
 
 int fstat(int fd, StatT *buf) {
   HANDLE h = reinterpret_cast<HANDLE>(_get_osfhandle(fd));
   return stat_handle(h, buf);
 }
 
+int mkdir(const wchar_t *path, int permissions) {
+  (void)permissions;
+  return _wmkdir(path);
+}
+
+int symlink_file_dir(const wchar_t *oldname, const wchar_t *newname,
+                     bool is_dir) {
+  DWORD flags = is_dir ? SYMBOLIC_LINK_FLAG_DIRECTORY : 0;
+  if (CreateSymbolicLinkW(newname, oldname,
+                          flags | SYMBOLIC_LINK_FLAG_ALLOW_UNPRIVILEGED_CREATE))
+    return 0;
+  int e = GetLastError();
+  if (e != ERROR_INVALID_PARAMETER)
+    return set_errno(e);
+  if (CreateSymbolicLinkW(newname, oldname, flags))
+    return 0;
+  return set_errno();
+}
+
+int symlink_file(const wchar_t *oldname, const wchar_t *newname) {
+  return symlink_file_dir(oldname, newname, false);
+}
+
+int symlink_dir(const wchar_t *oldname, const wchar_t *newname) {
+  return symlink_file_dir(oldname, newname, true);
+}
+
+int link(const wchar_t *oldname, const wchar_t *newname) {
+  if (CreateHardLinkW(newname, oldname, nullptr))
+    return 0;
+  return set_errno();
+}
+
+int remove(const wchar_t *path) {
+  detail::WinHandle h(path, DELETE, FILE_FLAG_OPEN_REPARSE_POINT);
+  if (!h)
+    return set_errno();
+  FILE_DISPOSITION_INFO info;
+  info.DeleteFile = TRUE;
+  if (!SetFileInformationByHandle(h, FileDispositionInfo, &info, sizeof(info)))
+    return set_errno();
+  return 0;
+}
+
+int truncate_handle(HANDLE h, off_t length) {
+  LARGE_INTEGER size_param;
+  size_param.QuadPart = length;
+  if (!SetFilePointerEx(h, size_param, 0, FILE_BEGIN))
+    return set_errno();
+  if (!SetEndOfFile(h))
+    return set_errno();
+  return 0;
+}
+
+int ftruncate(int fd, off_t length) {
+  HANDLE h = reinterpret_cast<HANDLE>(_get_osfhandle(fd));
+  return truncate_handle(h, length);
+}
+
+int truncate(const wchar_t *path, off_t length) {
+  detail::WinHandle h(path, GENERIC_WRITE, 0);
+  if (!h)
+    return set_errno();
+  return truncate_handle(h, length);
+}
+
+int rename(const wchar_t *from, const wchar_t *to) {
+  if (!(MoveFileExW(from, to,
+                    MOVEFILE_COPY_ALLOWED | MOVEFILE_REPLACE_EXISTING |
+                        MOVEFILE_WRITE_THROUGH)))
+    return set_errno();
+  return 0;
+}
+
+template <class... Args> int open(const wchar_t *filename, Args... args) {
+  return _wopen(filename, args...);
+}
+int close(int fd) { return _close(fd); }
+int chdir(const wchar_t *path) { return _wchdir(path); }
 #else
+int symlink_file(const char *oldname, const char *newname) {
+  return ::symlink(oldname, newname);
+}
+int symlink_dir(const char *oldname, const char *newname) {
+  return ::symlink(oldname, newname);
+}
+using ::chdir;
+using ::close;
 using ::fstat;
+using ::ftruncate;
+using ::link;
 using ::lstat;
+using ::mkdir;
+using ::open;
+using ::remove;
+using ::rename;
 using ::stat;
+using ::truncate;
 #endif
 
 
 struct FileDescriptor {
   const path& name;
   int fd = -1;
   StatT m_stat;
   file_status m_status;
 
   template <class... Args>
   static FileDescriptor create(const path* p, error_code& ec, Args... args) {
     ec.clear();
     int fd;
-    if ((fd = ::open(p->c_str(), args...)) == -1) {
+    if ((fd = detail::open(p->c_str(), args...)) == -1) {
       ec = capture_errno();
       return FileDescriptor{p};
     }
     return FileDescriptor(p, fd);
   }
 
   template <class... Args>
   static FileDescriptor create_with_status(const path* p, error_code& ec,
struct FileDescriptor {
   StatT const& get_stat() const { return m_stat; }
 
   bool status_known() const { return _VSTD_FS::status_known(m_status); }
 
   file_status refresh_status(error_code& ec);
 
   void close() noexcept {
     if (fd != -1)
-      ::close(fd);
+      detail::close(fd);
     fd = -1;
   }
 
   FileDescriptor(FileDescriptor&& other)
       : name(other.name), fd(other.fd), m_stat(other.m_stat),
         m_status(other.m_status) {
     other.fd = -1;
     other.m_status = file_status{};

 
 file_status posix_lstat(path const& p, error_code* ec) {
   StatT path_stat;
   return posix_lstat(p, path_stat, ec);
 }
 
 // http://pubs.opengroup.org/onlinepubs/9699919799/functions/ftruncate.html
 bool posix_ftruncate(const FileDescriptor& fd, off_t to_size, error_code& ec) {
-  if (::ftruncate(fd.fd, to_size) == -1) {
+  if (detail::ftruncate(fd.fd, to_size) == -1) {
     ec = capture_errno();
     return true;
   }
   ec.clear();
   return false;
 }
 
 bool posix_fchmod(const FileDescriptor& fd, const StatT& st, error_code& ec) {

 } // end namespace detail
 
 bool __copy_file(const path& from, const path& to, copy_options options,
                  error_code* ec) {
   using detail::FileDescriptor;
   ErrorHandler<bool> err("copy_file", ec, &to, &from);
 
   error_code m_ec;
-  FileDescriptor from_fd =
-      FileDescriptor::create_with_status(&from, m_ec, O_RDONLY | O_NONBLOCK);
+  FileDescriptor from_fd = FileDescriptor::create_with_status(
+      &from, m_ec, O_RDONLY | O_NONBLOCK | O_BINARY);
   if (m_ec)
     return err.report(m_ec);
 
   auto from_st = from_fd.get_status();
   StatT const& from_stat = from_fd.get_stat();
   if (!is_regular_file(from_st)) {
     if (not m_ec)
       m_ec = make_error_code(errc::not_supported);
if (!to_exists || overwrite_existing)
       return true;
     return err.report(errc::file_exists);
   }();
   if (!ShouldCopy)
     return false;
 
   // Don't truncate right away. We may not be opening the file we originally
   // looked at; we'll check this later.
-  int to_open_flags = O_WRONLY;
+  int to_open_flags = O_WRONLY | O_BINARY;
   if (!to_exists)
     to_open_flags |= O_CREAT;
   FileDescriptor to_fd = FileDescriptor::create_with_status(
       &to, m_ec, to_open_flags, from_stat.st_mode);
   if (m_ec)
     return err.report(m_ec);
 
   if (to_exists) {
if (!parent.empty()) {
     }
   }
   return __create_directory(p, ec);
 }
 
 bool __create_directory(const path& p, error_code* ec) {
   ErrorHandler<bool> err("create_directory", ec, &p);
 
-  if (::mkdir(p.c_str(), static_cast<int>(perms::all)) == 0)
+  if (detail::mkdir(p.c_str(), static_cast<int>(perms::all)) == 0)
     return true;
   if (errno != EEXIST)
     err.report(capture_errno());
   return false;
 }
 
 bool __create_directory(path const& p, path const& attributes, error_code* ec) {
   ErrorHandler<bool> err("create_directory", ec, &p, &attributes);
 
   StatT attr_stat;
   error_code mec;
   auto st = detail::posix_stat(attributes, attr_stat, &mec);
   if (!status_known(st))
     return err.report(mec);
   if (!is_directory(st))
     return err.report(errc::not_a_directory,
                       "the specified attribute path is invalid");
 
-  if (::mkdir(p.c_str(), attr_stat.st_mode) == 0)
+  if (detail::mkdir(p.c_str(), attr_stat.st_mode) == 0)
     return true;
   if (errno != EEXIST)
     err.report(capture_errno());
   return false;
 }
 
 void __create_directory_symlink(path const& from, path const& to,
                                 error_code* ec) {
   ErrorHandler<void> err("create_directory_symlink", ec, &from, &to);
-  if (::symlink(from.c_str(), to.c_str()) != 0)
+  if (detail::symlink_dir(from.c_str(), to.c_str()) == -1)
     return err.report(capture_errno());
 }
 
 void __create_hard_link(const path& from, const path& to, error_code* ec) {
   ErrorHandler<void> err("create_hard_link", ec, &from, &to);
-  if (::link(from.c_str(), to.c_str()) == -1)
+  if (detail::link(from.c_str(), to.c_str()) == -1)
     return err.report(capture_errno());
 }
 
 void __create_symlink(path const& from, path const& to, error_code* ec) {
   ErrorHandler<void> err("create_symlink", ec, &from, &to);
-  if (::symlink(from.c_str(), to.c_str()) == -1)
+  if (detail::symlink_file(from.c_str(), to.c_str()) == -1)
     return err.report(capture_errno());
 }
 
 path __current_path(error_code* ec) {
   ErrorHandler<path> err("current_path", ec);
 
   auto size = ::pathconf(".", _PC_PATH_MAX);
   _LIBCPP_ASSERT(size >= 0, "pathconf returned a 0 as max size");
 
   auto buff = unique_ptr<char[]>(new char[size + 1]);
   char* ret;
   if ((ret = ::getcwd(buff.get(), static_cast<size_t>(size))) == nullptr)
     return err.report(capture_errno(), "call to getcwd failed");
 
   return {buff.get()};
 }
 
 void __current_path(const path& p, error_code* ec) {
   ErrorHandler<void> err("current_path", ec, &p);
-  if (::chdir(p.c_str()) == -1)
+  if (detail::chdir(p.c_str()) == -1)
     err.report(capture_errno());
 }
 
 bool __equivalent(const path& p1, const path& p2, error_code* ec) {
   ErrorHandler<bool> err("equivalent", ec, &p1, &p2);
 
   error_code ec1, ec2;
   StatT st1 = {}, st2 = {};
#endif
   if (static_cast<size_t>(ret) >= size)
     return err.report(errc::value_too_large);
   buff[ret] = 0;
   return {buff.get()};
 }
 
 bool __remove(const path& p, error_code* ec) {
   ErrorHandler<bool> err("remove", ec, &p);
-  if (::remove(p.c_str()) == -1) {
+  if (detail::remove(p.c_str()) == -1) {
     if (errno != ENOENT)
       err.report(capture_errno());
     return false;
   }
   return true;
 }
 
 namespace {
if (mec == errc::no_such_file_or_directory)
       return 0;
     return err.report(mec);
   }
   return count;
 }
 
 void __rename(const path& from, const path& to, error_code* ec) {
   ErrorHandler<void> err("rename", ec, &from, &to);
-  if (::rename(from.c_str(), to.c_str()) == -1)
+  if (detail::rename(from.c_str(), to.c_str()) == -1)
     err.report(capture_errno());
 }
 
 void __resize_file(const path& p, uintmax_t size, error_code* ec) {
   ErrorHandler<void> err("resize_file", ec, &p);
-  if (::truncate(p.c_str(), static_cast< ::off_t>(size)) == -1)
+  if (detail::truncate(p.c_str(), static_cast< ::off_t>(size)) == -1)
     return err.report(capture_errno());
 }
 
 space_info __space(const path& p, error_code* ec) {
   ErrorHandler<void> err("space", ec, &p);
   space_info si;
   struct statvfs m_svfs = {};
   if (::statvfs(p.c_str(), &m_svfs) == -1) {