labath added a comment.

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.

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).

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...



================
Comment at: source/Host/common/FileSystem.cpp:253
+static int GetOpenFlags(uint32_t options) {
+  const bool read = options & File::OpenOptions::eOpenOptionRead;
+  const bool write = options & File::OpenOptions::eOpenOptionWrite;
----------------
`File::eOpenOptionRead` should be sufficient, no?


================
Comment at: source/Host/posix/FileSystem.cpp:74-79
+FILE *FileSystem::Fopen(const char *path, const char *mode) const {
   return ::fopen(path, mode);
 }
 
-int FileSystem::Open(const char *path, int flags, int mode) {
+int FileSystem::Open(const char *path, int flags, int mode) const {
   return ::open(path, flags, mode);
----------------
Why are these two `const`? It seems that at least with `eOpenOptionCanCreate`, 
one can create a new file, which is definitely not a "const" operation.


================
Comment at: source/Utility/FileSpec.cpp:321-322
 
+bool FileSpec::Empty() const { return m_directory && m_filename; }
+
 //------------------------------------------------------------------
----------------
We already have `operator bool` for this.


Repository:
  rLLDB LLDB

https://reviews.llvm.org/D54020



_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Reply via email to