[PATCH] D36265: Support lib changes to allow VFS implementation handling working dir locally.

Ilya Biryukov via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Wed Aug 16 01:01:28 PDT 2017


ilya-biryukov added a comment.

In https://reviews.llvm.org/D36265#832143, @zturner wrote:

> The main concern I have is that the Nt functions are not intended for public consumption.  They're considered internal, and as such they are subject to all the caveats listed here <https://msdn.microsoft.com/en-us/library/bb432200(v=vs.85).aspx>.


I totally agree that is not ideal. Even though `Nt` functions for file manipulations have been there for ages. I personally highly doubt they will go away in the foreseeable future, but there is no guarantee.

> I fear it would also be impossible to safely implement some of the functions with the Nt methods.  For example, `openFileForRead` returns an fd in one of its arguments.  The way this is implemented is by calling `CreateFile` and then taking the resulting `HANDLE` and calling `_open_osfhandle` on it.  Does this work when it's an `NtCreate` handle?  Maybe?  I don't know, but if we can't mix functions then this is just one of the problems.  The point is that some of our APIs expose these handles and descriptors to the outside world, and we can't expect external users to use the Nt APIs.

That is certainly an issue I overlooked. There are no indications if `_open_osfhandle` will work properly on handles created by `Nt` functions.

> If all this is hidden behind a separate function, then the requirements for that function could be documented independently.  Regardless, the main thing is that it's an internal API, and it seems like a very bad idea to have our entire filesystem library for all of LLVM be based entirely on unsupported, internal OS APIs.

Another thing is that for Windows we may get away with just converting the paths to absolute all the time. The symlinks changing their target rapidly (i.e. during a single compile) is something we're having trouble with only on Linux, and I doubt this use-case would ever come up on Windows.
My intention currently is to provide the `openat`-based APIs only on Unix that have a relevant library support.
The final aim to implement a `clang::vfs::FileSystem` that does not rely on process-wide working directory. It would either be based on top of `openat`-based APIs or would convert paths to absolute on each request.

In https://reviews.llvm.org/D36265#842612, @krytarowski wrote:

> Almost certainly nobody maintains newer LLVM on older versions of mentioned BSD systems.


Good to know, thanks.


https://reviews.llvm.org/D36265





More information about the llvm-commits mailing list