[libcxx-commits] [PATCH] D91170: [15/N] [libcxx] Implement the canonical function for windows

[Adrian McCarthy via Phabricator via libcxx-commits]

amccarth requested changes to this revision.
amccarth added inline comments.
This revision now requires changes to proceed.


================
Comment at: libcxx/src/filesystem/operations.cpp:863
+  if (!GetFinalPathNameByHandleW(h, ret, MAX_PATH,
+                                 FILE_NAME_NORMALIZED | VOLUME_NAME_DOS))
+    return err.report(detail::make_windows_error(GetLastError()));
----------------
This return value needs to be checked more carefully.

If the final path is longer than the buffer, then `GetFinalPathNameByHandleW` will return the actual length needed, which will convince this if statement that the call succeeded when it didn't, and then we'll process the uninitialized buffer.

How can the final path require a buffer longer than `MAX_PATH`?  Two ways:

1.  The function always returns the `\\?\` prefix, so a path that (with it's terminating `L'\0'`) is exactly `MAX_PATH` will require `MAX_PATH+4` when the prefix is added.

2.  `MAX_PATH` applies to most Windows APIs, but not to the actual filesystem.  It's possible to have a final path that's longer than `MAX_PATH` (especially since it's using the `\\?\` prefix).  If we don't think it's worth handling those, fine, but we'd need to detect that case so that we can return an error.  (A common way this happens is when the user uses `subst` to replace a long prefix with a fictitious drive letter.  This is, in fact, how we get certain LLDB source paths under the `MAX_PATH` limit on some of the Windows build bots.)


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

https://reviews.llvm.org/D91170