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

[Hubert Tong via Phabricator via libcxx-commits]

hubert.reinterpretcast added inline comments.


================
Comment at: libcxx/src/filesystem/operations.cpp:545
+  #if defined(__MVS__)
+    char buff[1024 +1];
+  #else
----------------
ldionne wrote:
> 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?
re: EINVAL when using NULL for the second argument, see:
https://www.ibm.com/support/knowledgecenter/en/SSLTBW_2.4.0/com.ibm.zos.v2r4.bpxbd00/rpath.htm

re: ENAMETOOLONG, the fixed limits is documented for what could be thought of as the "system call interface":
https://www.ibm.com/support/knowledgecenter/en/SSLTBW_2.4.0/com.ibm.zos.v2r4.bpxb100/rph.htm

re: deployment goals, I am not in a position to provide that information (@zibi, can you say more?)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92110