[libcxx-commits] [PATCH] D153037: [libcxx] Migrate posix_compat.h layer to not use CRT filesystem functions.

Thu Jun 15 13:23:07 PDT 2023

mstorsjo accepted this revision.
mstorsjo added a comment.

This looks good to me, thanks! I guess it's mostly up to whether others are ok with the kludge moved into `FileDescriptor::create`.

================
Comment at: libcxx/src/filesystem/posix_compat.h:353
+
+  size_t buff_size = MAX_PATH + 10;
+  std::unique_ptr<wchar_t, decltype(&::free)> buff(static_cast<wchar_t*>(malloc(buff_size * sizeof(wchar_t))), &::free);
----------------
jyknight wrote:
> Mordante wrote:
> > Please add comment why this magic value (10) is used. 
> I'm not sure why it was chosen...I copied that from realpath, below. It doesn't make a difference functionally, since if we go over, we simply reallocate.
I don't quite remember my reasoning or whether this was something I made (feels plausible) or that I was inspired to from somewhere else. I guess it was some sort of fudge factor.

`MAX_PATH` isn't an absolute max for all paths on Windows in all cases anyway - but I wanted to make sure that most paths that are dealt with do fit in here, even if they are `MAX_PATH` + null terminator. (I don't remember if paths that are limited by `MAX_PATH` include the null terminator in that size or not.) So for that purpose, maybe `MAX_PATH+1` could be just as good. But I guess I chose 10 for some extra margin...

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D153037