[libcxx] Migrate posix_compat.h layer to not use CRT filesystem functions.
authorJames Y Knight <jyknight@google.com>
Mon, 19 Jun 2023 17:03:37 +0000 (13:03 -0400)
committerJames Y Knight <jyknight@google.com>
Wed, 5 Jul 2023 17:27:46 +0000 (13:27 -0400)
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).

Reviewed By: #libc, mstorsjo, Mordante

Differential Revision: https://reviews.llvm.org/D153037

libcxx/src/filesystem/file_descriptor.h
libcxx/src/filesystem/posix_compat.h

index 647fb98..d3a668f 100644 (file)
@@ -123,7 +123,25 @@ struct FileDescriptor {
   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};
     }
@@ -148,8 +166,14 @@ struct FileDescriptor {
   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;
   }
 
index 9997db7..f11f415 100644 (file)
@@ -204,7 +204,9 @@ inline int fstat(int fd, StatT *buf) {
 
 inline int mkdir(const wchar_t *path, int permissions) {
   (void)permissions;
-  return _wmkdir(path);
+  if (!CreateDirectoryW(path, nullptr))
+    return set_errno();
+  return 0;
 }
 
 inline int symlink_file_dir(const wchar_t *oldname, const wchar_t *newname,
@@ -279,11 +281,11 @@ inline int rename(const wchar_t *from, const wchar_t *to) {
   return 0;
 }
 
-template <class... Args> int open(const wchar_t *filename, Args... args) {
-  return _wopen(filename, args...);
+inline int chdir(const wchar_t* path) {
+  if (!SetCurrentDirectoryW(path))
+    return set_errno();
+  return 0;
 }
-inline int close(int fd) { return _close(fd); }
-inline int chdir(const wchar_t *path) { return _wchdir(path); }
 
 struct StatVFS {
   uint64_t f_frsize;
@@ -318,7 +320,25 @@ inline int statvfs(const wchar_t *p, StatVFS *buf) {
   return 0;
 }
 
-inline wchar_t *getcwd(wchar_t *buff, size_t size) { return _wgetcwd(buff, size); }
+inline wchar_t* getcwd([[maybe_unused]] wchar_t* in_buf, [[maybe_unused]] 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();
+}
 
 inline wchar_t *realpath(const wchar_t *path, [[maybe_unused]] wchar_t *resolved_name) {
   // Only expected to be used with us allocating the buffer.
@@ -461,7 +481,6 @@ inline 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;
@@ -472,7 +491,6 @@ using ::getcwd;
 using ::link;
 using ::lstat;
 using ::mkdir;
-using ::open;
 using ::readlink;
 using ::realpath;
 using ::remove;