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

[Zachary Turner via Phabricator via llvm-commits]

zturner added a comment.

In https://reviews.llvm.org/D36265#831534, @ilya-biryukov wrote:

> Technically, creating separate functions should not be too hard. 
>  There are two concerns that I have with having both APIs:
>
> 1. We must be very careful to not mix `Win32` and `Nt` APIs (i.e. file opened via `NtCreateFile` should be read via `NtReadFile`, not `ReadFile`).
> 2. Having both `Win32` and `Nt` functions  is that `Nt` variants may stay untested if most of LLVM and Clang will be using `Win32`. What problems do you foresee if we switch to `Nt` API everywhere?


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

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.


https://reviews.llvm.org/D36265