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

[Zbigniew Sarbinowski via Phabricator via libcxx-commits]

zibi marked 3 inline comments as done.
zibi 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:
> > > 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?)
Unfortunately we do not have the safe version of `realpath` and the value of `_POSIZ_VERSOIN` is much lower.  The deployment goals are still in flux.

Here is another proposal:
```
#if defines(__MVS__) && !defined(PATH_MAX) 
  char buff[ _XOPEN_PATH_MAX + 1 ];
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92110