[PATCH] D58835: [Support] Treat truncation of fullpath as error

[Eli Friedman via Phabricator via llvm-commits]

efriedma accepted this revision.
efriedma added a comment.
This revision is now accepted and ready to land.

LGTM



================
Comment at: llvm/lib/Support/Unix/Path.inc:114
+  if (chars >= PATH_MAX)
+    return 1;
   if (!realpath(fullpath, ret))
----------------
Hahnfeld wrote:
> zturner wrote:
> > efriedma wrote:
> > > The use of PATH_MAX here seems dubious; in practice, many targets support paths longer than PATH_MAX. Probably should just use a raw_string_ostream or something like that instead of snprintf to a fixed buffer.
> > Even better, `llvm::sys::path::join()`.  I'm not sure why all of these global functions operate on raw pointers instead of LLVM's string ADTs.
> I played a bit on my Arch Linux system: It's indeed possible to create paths that are longer than `PATH_MAX = 4096`. But I can't get the `realpath` nor even `cd` into there, and I'd guess that many more tools are broken.
> 
> In that case I think there's no difference between fixed buffers and the string ADTs, except that the latter are probably slower.
I guess this is okay given the way the code is currently written.

It should probably be rewritten eventually to work in a more consistent way on systems which don't have a PATH_MAX, like Hurd, but that shouldn't block this patch, I think.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D58835