[Lldb-commits] [PATCH] D54020: [FileSystem] Open File instances through the FileSystem.

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere added a comment.

In https://reviews.llvm.org/D54020#1285201, @labath wrote:

> I am not sure what do you mean by "translating paths" in the VFS. If you mean something like "return a `FILE *` for `/whatever/my_reproducer/vfs/bin/ls` when I ask for `/bin/ls`", then I think that's a good idea, as it's most likely to work with all of our existing code (e.g. mmapping). Although, in this case, i am not sure how much benefit will the llvm VFS bring to the game, as most of the operations could then be implemented by plainly prepending some  prefix to a given path.


Let me try to explain this better. This is mostly a question about what kind of API the VFS (which lives in LLVM) provides to deal with files in lldb (i.e. through `FILE*` and file descriptors).

The first possibility is providing a method in the VFS that takes a "virtual" path and returns the "underlying" path. Something like  `Optional<path> vfs::getUnderlyingPath(path)`. This doesn't just have to be a prefix thing but you are right that it's mostly going to be just that. The problem is that this method wouldn't work for virtual file systems that don't have files on disk. Hence the lack of generality.

The second possibility is providing a method in the VFS that returns `FILE*`. When I was writing this I was still hoping that there was something like the fake `FILE*` you mentioned below. If that were the case it could support virtual file systems that have files that strictly live in memory. But if this doesn't work for Windows it's not really any better than the first alternative.

> Getting rid of `FILE *` is not going to be easy, as it is used for some of our external dependencies (libedit). It's possible to create a fake FILE* on posix systems (`fopencookie, fmemopen`), but I don't think it's possible to do something like that on windows (which is why I stopped when I was looking at this some time ago).

Yup, this is not something I'm proposing.

> Also, be aware that there are some places where we open `raw_ostream`s directly (`Debugger::EnableLog` comes to mind). I guess those will need to go through the `FileSystem` too...

Yup, we need to audit all file access. This particular example actually goes through file so it should be covered by this change.


Repository:
  rLLDB LLDB

https://reviews.llvm.org/D54020