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

[Mark de Wever via Phabricator via libcxx-commits]

Mordante requested changes to this revision.
Mordante added a comment.
This revision now requires changes to proceed.

Can you rebase the patch on main and fix the CI so we can validate the patch works?



================
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);
----------------
mstorsjo wrote:
> 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...
Ah I missed it was copy pasted. This is why I dislike magic, nobody remembers the secret invocation afterwards ;-)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D153037