[libcxx] Migrate posix_compat.h layer to not use CRT filesystem functions.

[jyknight]

Summary

Right now, the filesystem APIs _mostly_ use Win32 API calls, but a
small number of functions use CRT APIs instead. The semantics are
effectively the same, except for which sorts of error codes are
returned. We want to be consistent about returning only native Win32
error codes, as a prerequisite for https://reviews.llvm.org/D151493.

This change switches getcwd, chdir, and mkdir. It does _not_ switch
open/close, because there are difficulties around the use of C-runtime
file descriptor numbers. Instead, those two APIs are removed from
posix_compat.h, and the win32-specific code inlined into the
operations.cpp FileDescriptor class (with a TODO comment).

[Mordante]

This patch will get conflicts when D152377 is merged.

libcxx/src/filesystem/posix_compat.h

353

Please add comment why this magic value (10) is used.

[jyknight]

libcxx/src/filesystem/posix_compat.h

353

I'm not sure why it was chosen...I copied that from realpath, below. It doesn't make a difference functionally, since if we go over, we simply reallocate.

[mstorsjo]

This looks good to me, thanks! I guess it's mostly up to whether others are ok with the kludge moved into FileDescriptor::create.

libcxx/src/filesystem/posix_compat.h

353

I don't quite remember my reasoning or whether this was something I made (feels plausible) or that I was inspired to from somewhere else. I guess it was some sort of fudge factor.

MAX_PATH isn't an absolute max for all paths on Windows in all cases anyway - but I wanted to make sure that most paths that are dealt with do fit in here, even if they are MAX_PATH + null terminator. (I don't remember if paths that are limited by MAX_PATH include the null terminator in that size or not.) So for that purpose, maybe MAX_PATH+1 could be just as good. But I guess I chose 10 for some extra margin...

[Mordante]

Can you rebase the patch on main and fix the CI so we can validate the patch works?

libcxx/src/filesystem/posix_compat.h

353

Ah I missed it was copy pasted. This is why I dislike magic, nobody remembers the secret invocation afterwards ;-)

[jyknight]

Rebase.

[Mordante]

LGTM!

[jyknight]

Rebase to rerun tests before submitting.

[ldionne]

libcxx/src/filesystem/file_descriptor.h

127–138 ↗

(On Diff #537321)

Is there anything blocking us from doing this now? I'm a bit concerned that this patch is adding a TODO without much of an intent to remove it (unless I'm mistaken).

[jyknight]

libcxx/src/filesystem/file_descriptor.h

127–138 ↗

(On Diff #537321)

There is nothing blocking it, other than someone's time to work on it. I was indeed not planning to volunteer to do it at the moment, but would be happy if someone else wants to tackle it.

(I'll also note that CopyFile2 ought to be used on windows, regardless; the TODO just documents that deficiency).

libcxx/src/filesystem/operations.cpp

struct FileDescriptor {
   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 = detail::open(p->c_str(), args...)) == -1) {
+#ifdef _LIBCPP_WIN32API
+    // TODO: most of the filesystem implementation uses native Win32 calls
+    // (mostly via posix_compat.h). However, here we use the C-runtime APIs to
+    // open a file, because we subsequently pass the C-runtime fd to
+    // `std::[io]fstream::__open(int fd)` in order to implement copy_file.
+    //
+    // Because we're calling the windows C-runtime, win32 error codes are
+    // translated into C error numbers by the C runtime, and returned in errno,
+    // rather than being accessible directly via GetLastError.
+    //
+    // Ideally copy_file should be calling the Win32 CopyFile2 function, which
+    // works on paths, not open files -- at which point this FileDescriptor type
+    // will no longer be needed on windows at all.
+    fd = ::_wopen(p->c_str(), args...);
+#else
+    fd = open(p->c_str(), args...);
+#endif
+
+    if (fd == -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,
                                            Args... args) {
     FileDescriptor fd = create(p, ec, args...);
     if (!ec)
       fd.refresh_status(ec);
 
     return fd;
   }
 
   file_status get_status() const { return m_status; }
   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)
-      detail::close(fd);
+    if (fd != -1) {
+#ifdef _LIBCPP_WIN32API
+      ::_close(fd);
+#else
+      ::close(fd);
+#endif
+      // FIXME: shouldn't this return an error_code?
+    }
     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{};

libcxx/src/filesystem/posix_compat.h

 
 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);
+  if (!CreateDirectoryW(path, nullptr))
+    return set_errno();
+  return 0;
 }
 
 int symlink_file_dir(const wchar_t *oldname, const wchar_t *newname,
                      bool is_dir) {
   path dest(oldname);
   dest.make_preferred();
   oldname = dest.c_str();
   DWORD flags = is_dir ? SYMBOLIC_LINK_FLAG_DIRECTORY : 0;

 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 chdir(const wchar_t* path) {
+  if (!SetCurrentDirectoryW(path))
+    return set_errno();
+  return 0;
 }
-int close(int fd) { return _close(fd); }
-int chdir(const wchar_t *path) { return _wchdir(path); }
 
 struct StatVFS {
   uint64_t f_frsize;
   uint64_t f_blocks;
   uint64_t f_bfree;
   uint64_t f_bavail;
 };
 
if (!GetDiskFreeSpaceExW(dir.c_str(), &free_bytes_available_to_caller,
     return set_errno();
   buf->f_frsize = 1;
   buf->f_blocks = total_number_of_bytes.QuadPart;
   buf->f_bfree = total_number_of_free_bytes.QuadPart;
   buf->f_bavail = free_bytes_available_to_caller.QuadPart;
   return 0;
 }
 
-wchar_t *getcwd(wchar_t *buff, size_t size) { return _wgetcwd(buff, size); }
+wchar_t* getcwd(wchar_t* in_buf, size_t in_size) {
+  // Only expected to be used with us allocating the buffer.
+  _LIBCPP_ASSERT(in_buf == nullptr, "Windows getcwd() assumes in_buf==nullptr");
+  _LIBCPP_ASSERT(in_size == 0, "Windows getcwd() assumes in_size==0");
+
+  size_t buff_size = MAX_PATH + 10;
+  std::unique_ptr<wchar_t, decltype(&::free)> buff(static_cast<wchar_t*>(malloc(buff_size * sizeof(wchar_t))), &::free);
+  DWORD retval = GetCurrentDirectoryW(buff_size, buff.get());
+  if (retval > buff_size) {
+    buff_size = retval;
+    buff.reset(static_cast<wchar_t*>(malloc(buff_size * sizeof(wchar_t))));
+    retval = GetCurrentDirectoryW(buff_size, buff.get());
+  }
+  if (!retval) {
+    set_errno();
+    return nullptr;
+  }
+  return buff.release();
+}
 
 wchar_t *realpath(const wchar_t *path, wchar_t *resolved_name) {
   // Only expected to be used with us allocating the buffer.
   _LIBCPP_ASSERT(resolved_name == nullptr,
                  "Windows realpath() assumes a null resolved_name");
 
   WinHandle h(path, FILE_READ_ATTRIBUTES, 0);
   if (!h) {

 #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 ::fchmod;
 #if defined(AT_SYMLINK_NOFOLLOW) && defined(AT_FDCWD)
 using ::fchmodat;
 #endif
 using ::fstat;
 using ::ftruncate;
 using ::getcwd;
 using ::link;
 using ::lstat;
 using ::mkdir;
-using ::open;
 using ::readlink;
 using ::realpath;
 using ::remove;
 using ::rename;
 using ::stat;
 using ::statvfs;
 using ::truncate;