On Feb 14, 2014, at 8:00 AM, Manuel Klimek <[email protected]> wrote:
> On Fri, Feb 14, 2014 at 4:15 PM, Ben Langmuir <[email protected]> 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? I don’t think this design is limiting, because if it wants, a multiplexer can always claim to be the file’s owner and then it will be called for these *OpenFIle operations. That said, I am trying to optimize the interface for what I think is the more common case, where the multiplexer is not involved in operations on the open file and would just pass them to the file system that owns the file anyway. This is the case for the overlay file system I have in mind. But maybe I’m misguided - do you have a use-case for the multiplexer customizing these operations? > > 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 >
_______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
