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

Martin Storsjö via Phabricator via libcxx-commits libcxx-commits at lists.llvm.org
Fri Dec 4 04:05:58 PST 2020

mstorsjo added inline comments.

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()));
amccarth wrote:
> 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.)
Thanks, good catch. Will update with a stack buffer that should fit all practical cases (MAX_PATH+10) and fall back on dynamic allocation if that wasn't big enough.



More information about the libcxx-commits mailing list