[libcxx-commits] [PATCH] D91172: [17/N] [libcxx] Implement the read_symlink function for windows

[Adrian McCarthy via Phabricator via libcxx-commits]

amccarth accepted this revision.
amccarth added a comment.

LGTM.



================
Comment at: libcxx/src/filesystem/operations.cpp:1495
+#if defined(_LIBCPP_WIN32API)
+  uint8_t buf[MAXIMUM_REPARSE_DATA_BUFFER_SIZE + sizeof(wchar_t)];
+  detail::WinHandle h(p.c_str(), FILE_READ_ATTRIBUTES,
----------------
mstorsjo wrote:
> amccarth wrote:
> > What's the purpose of the `+ sizeof(wchar_t)` here?
> Not entirely sure - I based it on this: https://github.com/microsoft/STL/blob/master/stl/inc/filesystem#L3198
> 
> However https://docs.microsoft.com/en-us/windows-hardware/drivers/ifs/fsctl-get-reparse-point says
> 
> > For a REPARSE_DATA_BUFFER structure, this value must be at least REPARSE_DATA_BUFFER_HEADER_SIZE, plus the size of the expected user-defined data, and it must be less than or equal to MAXIMUM_REPARSE_DATA_BUFFER_SIZE.
> 
> So with that in mind, it actually seems incorrect to have it be any larger than MAXIMUM_REPARSE_DATA_BUFFER_SIZE. So I guess I should change it to that. But that use in MS STL seems curious...
> 
> Or is that stated from the perspective of the implementation of FSCTL_GET_REPARSE_POINT, saying that the function may only ever set/use up to MAXIMUM_REPARSE_DATA_BUFFER_SIZE bytes of output space, regardless of how large a buffer is allocated?
Yes, I think MAXIMUM_REPARSE_DATA_BUFFER_SIZE is the most it'll ever write to the buffer, no matter how large you say the buffer is.  I think the corresponding FSCTL_SET_REPARSE_POINT will actually fail if you try to send too much data.

My guess is that the `+ sizeof(wchar_t)` was so that a zero-terminator could be added should anyone every inadvertently treat the buffer as a string.

So I guess this all works, but I wisth


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91172