[libcxx-commits] [PATCH] D92110: [SystemZ][ZOS] Provide PATH_MAX macro for libcxx

[Louis Dionne via Phabricator via libcxx-commits]

ldionne added inline comments.


================
Comment at: libcxx/src/filesystem/operations.cpp:545
+  #if defined(__MVS__)
+    char buff[1024 +1];
+  #else
----------------
hubert.reinterpretcast wrote:
> ldionne wrote:
> > hubert.reinterpretcast wrote:
> > > Minor nit: style.
> > Hardcoding buffer sizes like that is a great way to trigger a buffer overflow when assumptions are not respected. I would much rather we don't introduce this sort of code in libc++, to be honest. Is there a reason why zOS can't just provide `PATH_MAX`?
> > 
> > Or, even much better, why doesn't zOS satisfy `defined(_POSIX_VERSION) && _POSIX_VERSION >= 200112`? Then, we could use the safe and non-buggy version of `realpath`. The buffer-taking version is documented in various places, including `realpath`'s manpage itself, as being buggy.
> > Hardcoding buffer sizes like that is a great way to trigger a buffer overflow when assumptions are not respected.
> 
> Precisely because violating the assumptions mean that buffer overflow would be introduced into compiled programs, changes that would cause the assumption to not be respected cannot realistically be made.
> 
> > I would much rather we don't introduce this sort of code in libc++, to be honest. Is there a reason why zOS can't just provide `PATH_MAX`?
> 
> The POSIX specification provides rationale for not defining PATH_MAX in <limits.h>.
> 
> > Or, even much better, why doesn't zOS satisfy `defined(_POSIX_VERSION) && _POSIX_VERSION >= 200112`? Then, we could use the safe and non-buggy version of `realpath`. The buffer-taking version is documented in various places, including `realpath`'s manpage itself, as being buggy.
> 
> Deployed systems have stability considerations that may be incompatible with such a change. The IBM documentation for released versions clearly state that passing NULL to the second parameter produces an error. Indeed the bugginess of the buffer-taking version is why the hardcoded 1024 that the implementation of `realpath` on z/OS uses is preferable to an implementation that does not implement a fixed upper bound on the buffer-taking form of `realpath`.
> 
What are your deployment goals? What versions of z/OS do you want libc++ to work on? Also, where's the documentation you're referring to?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92110