[PATCH] D51641: [VFS] Cache the current working directory for the real FS.

Pavel Labath via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Wed Jan 9 08:46:48 PST 2019


labath added a comment.

I wholeheartedly support an openat(2) based VFS, as the current one falls short of the expectations you have of it and is pretty broken right now.

As for your assumption #2, I think that depends on what does one want to get out of the VFS. I can certainly see a use for having a  thread-local (VFS-local) CWD. In fact I remember seeing some code in LLDB, which used some darwin-only interface to create a real thread-local CWD in one function. However, I can also see a use for a VFS which is so "real" that it shares the notion of the CWD with the OS. What could happen in lldb is that a user sets the CWD in python (using the proper python api), and then passes a relative path to (lib)lldb. Since lldb never advertised to have any notion of a "lldb-local" CWD, the user would (righfully) expect that that path is resolved relative to the CWD he has just set. And that's what happened until we started using VFS. In fact, it still happens now, because the VFS is so bad at having a local CWD. So, the only reason we actually discovered this was because  VFS->getCurrentWorkingDirectory returned an empty string if a it's CWD has never been set. This later translated to a null pointer and a crash.

So it almost sounds to me like we should have two implementations of the VFS. A "real" one, which shared the CWD with the OS, and an "almost real" one, which has a local CWD. Another position would be to just say that lldb was wrong to use the VFS in the first place, as it does not promise we want (and it should use a different abstraction instead). That view would be supported by the fact that there are other interface disconnects relating to the use of VFS in lldb (FILE* and friends). However, that's something you and Jonas will have to figure out (I'm just the peanut gallery here :P).


Repository:
  rC Clang

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

https://reviews.llvm.org/D51641





More information about the cfe-commits mailing list