[libcxx-commits] [PATCH] D91169: [14/N] [libcxx] Implement the current_path function for windows

Martin Storsjö via Phabricator via libcxx-commits libcxx-commits at lists.llvm.org
Thu Dec 3 05:09:16 PST 2020


mstorsjo added inline comments.


================
Comment at: libcxx/src/filesystem/operations.cpp:1139
+#if defined(_LIBCPP_WIN32API)
+  std::unique_ptr<wchar_t, decltype(&::free)> buff(::_wgetcwd(nullptr, 0),
+                                                   &::free);
----------------
amccarth wrote:
> Is `_wgetcwd` the right choice here?  Shouldn't this instead go straight to Windows with `GetCurrentDirectoryW` so it doesn't take a dependence on MSVC's CRT?
I guess we could do that as well; we still have a few other calls that go through the CRT (in the FileDescriptor helper class further up in the file, used for file copying), and in this case, there's no other practical limitation in `_wgetcwd` that we'd get around, other than cleanliness. (Calling `GetCurrentDirectoryW` takes two calls and manual allocation inbetween, unless we want to hardcode a `PATH_MAX` allocation, but I'd rather move away from such specific limits.)

But yeah I can change it if you still prefer that.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D91169/new/

https://reviews.llvm.org/D91169



More information about the libcxx-commits mailing list