[PATCH] Some infrastructure work for virtual file system (now on phab)

Manuel Klimek klimek at google.com
Fri Feb 14 08:00:10 PST 2014


On Fri, Feb 14, 2014 at 4:15 PM, Ben Langmuir <blangmuir at apple.com> wrote:

>
>
> ================
> Comment at: lib/Basic/VirtualFileSystem.cpp:62-83
> @@ +61,24 @@
> +
> +ErrorOr<AbstractFileSystem::Status>
> +AbstractFileSystem::status(AbstractFileSystem::FileDescriptor FD) {
> +  // FIXME: when we support virtual files, use information from the FD to
> lookup
> +  // which AbstractFileSystem to perform this operation on.
> +  return getRealFileSystem()->statusOfOpenFile(FD);
> +}
> +
> +error_code AbstractFileSystem::getBufferForFile(
> +    FileDescriptor FD, const llvm::Twine &Name,
> +    llvm::OwningPtr<llvm::MemoryBuffer> &Result, int64_t FileSize,
> +    bool RequiresNullTerminator) {
> +  // FIXME: when we support virtual files, use information from the FD to
> lookup
> +  // which AbstractFileSystem to perform this operation on.
> +  return getRealFileSystem()->getBufferForOpenFile(FD, Name, Result,
> FileSize,
> +
> RequiresNullTerminator);
> +}
> +
> +error_code AbstractFileSystem::close(AbstractFileSystem::FileDescriptor
> FD) {
> +  // FIXME: when we support virtual files, use information from the FD to
> lookup
> +  // which AbstractFileSystem to perform this operation on.
> +  return getRealFileSystem()->closeOpenFile(FD);
> +}
> +
> ----------------
> Ben Langmuir wrote:
> > Manuel Klimek wrote:
> > > I'd have expected this->*OpenFile(...) calls in those methods.
> > These methods take a file descriptor as a parameter, so the file is
> already open.
> Oh sorry, I think I misunderstood your comment.
>
> The reason we don't call this->*OpenFile in here is that the
> FileDescriptor object (when we create one) should give us the
> AbstractFileSystem that we need to call the method on.  If we have a
> virtual file system composed of several AbstractFileSystems overlaid on top
> of each other, we shouldn't have to search for which file system owns the
> open file, since by opening it we already figured that out and can save it
> in the file descriptor.
>

I think I don't understand that from a design point of view - if we have a
file system implementation that "multiplexes" filesystems, I think it
should be *its* respsonsibility how to multiplex it. What are the reasons
we'd want that in the base class?

This design decision seems to currently just limit what you can do with an
abstract file system.

What am I missing?
/Manuel


>
>
> http://llvm-reviews.chandlerc.com/D2745
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20140214/07f3f17a/attachment.html>


More information about the cfe-commits mailing list