[PATCH] D88666: DirectoryWatcher: add an implementation for Windows

[Saleem Abdulrasool via Phabricator via cfe-commits]

compnerd added inline comments.


================
Comment at: clang/lib/DirectoryWatcher/windows/DirectoryWatcher-windows.cpp:87
+    DWORD dwLength = GetFinalPathNameByHandleW(hDirectory, NULL, 0, 0);
+    std::unique_ptr<WCHAR[]> buffer{new WCHAR[dwLength + 1]};
+    (void)GetFinalPathNameByHandleW(hDirectory, buffer.get(), dwLength + 1, 0);
----------------
compnerd wrote:
> amccarth wrote:
> > compnerd wrote:
> > > aaron.ballman wrote:
> > > > Is a smart pointer required here or could you use `std::vector<WCHAR>` and reserve the space that way?
> > > Sure, I can convert this to a `std::vector<WCHAR>` instead.
> > * I guess it's fine to use the array form of `std::unique_ptr` (but then you should `#include <memory>`).  If it were me, I'd probably just use a `std::wstring` or `std::vector<WCHAR>`.
> > 
> > * `dwLength` already includes the size of the null terminator.  Your first `GetFinalPathNameByHandleW` function "fails" because the buffer is too small.  The does says that, if it fails because the buffer is too small, then the return value is the required size _including_ the null terminator.  (In the success case, it's the size w/o the terminator.)
> > 
> > * I know this is the Windows-specific implementation, but it might be best to just the Support api ` realPathFromHandle`, which does this and has tests.
> I didn't know about `realPathFromHandle` - I prefer that actually.
Actually, `realPathFromHandle` is private to `Path.cpp` :-(


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88666