From 8a713038274208eadfcc14df9a8d78e94757551b Mon Sep 17 00:00:00 2001 From: Andreas Kling Date: Fri, 21 Dec 2018 17:28:16 +0100 Subject: [PATCH] Get rid of FS::read_inode_bytes() and use Inode::read_bytes() everywhere. --- VirtualFileSystem/Ext2FileSystem.cpp | 76 ----------------------- VirtualFileSystem/Ext2FileSystem.h | 1 - VirtualFileSystem/FileDescriptor.cpp | 3 +- VirtualFileSystem/FileSystem.cpp | 13 ++-- VirtualFileSystem/FileSystem.h | 2 - VirtualFileSystem/SyntheticFileSystem.cpp | 37 ----------- VirtualFileSystem/SyntheticFileSystem.h | 1 - VirtualFileSystem/VirtualFileSystem.cpp | 5 +- 8 files changed, 11 insertions(+), 127 deletions(-) diff --git a/VirtualFileSystem/Ext2FileSystem.cpp b/VirtualFileSystem/Ext2FileSystem.cpp index f428820a75..584694e814 100644 --- a/VirtualFileSystem/Ext2FileSystem.cpp +++ b/VirtualFileSystem/Ext2FileSystem.cpp @@ -404,82 +404,6 @@ ssize_t Ext2FSInode::read_bytes(Unix::off_t offset, size_t count, byte* buffer, return nread; } -ssize_t Ext2FS::read_inode_bytes(InodeIdentifier inode, Unix::off_t offset, size_t count, byte* buffer, FileDescriptor*) const -{ - ASSERT(offset >= 0); - ASSERT(inode.fsid() == id()); - - auto e2inode = lookup_ext2_inode(inode.index()); - if (!e2inode) { - kprintf("ext2fs: readInodeBytes: metadata lookup for inode %u failed\n", inode.index()); - return -EIO; - } - -#if 0 - // FIXME: We can't fail here while the directory traversal depends on this function. :] - if (isDirectory(e2inode->i_mode)) - return -EISDIR; -#endif - - if (e2inode->i_size == 0) - return 0; - - // Symbolic links shorter than 60 characters are store inline inside the i_block array. - // This avoids wasting an entire block on short links. (Most links are short.) - static const unsigned maxInlineSymlinkLength = 60; - if (isSymbolicLink(e2inode->i_mode) && e2inode->i_size < maxInlineSymlinkLength) { - ssize_t nread = min((Unix::off_t)e2inode->i_size - offset, static_cast(count)); - memcpy(buffer, e2inode->i_block + offset, nread); - return nread; - } - - // FIXME: It's grossly inefficient to fetch the blocklist on every call to readInodeBytes(). - // It needs to be cached! - auto list = block_list_for_inode(*e2inode); - if (list.is_empty()) { - kprintf("ext2fs: readInodeBytes: empty block list for inode %u\n", inode.index()); - return -EIO; - } - - dword firstBlockLogicalIndex = offset / blockSize(); - dword lastBlockLogicalIndex = (offset + count) / blockSize(); - if (lastBlockLogicalIndex >= list.size()) - lastBlockLogicalIndex = list.size() - 1; - - dword offsetIntoFirstBlock = offset % blockSize(); - - ssize_t nread = 0; - size_t remainingCount = min((Unix::off_t)count, (Unix::off_t)e2inode->i_size - offset); - byte* out = buffer; - -#ifdef EXT2_DEBUG - kprintf("ok let's do it, read(%llu, %u) -> blocks %u thru %u, oifb: %u\n", offset, count, firstBlockLogicalIndex, lastBlockLogicalIndex, offsetIntoFirstBlock); -#endif - - for (dword bi = firstBlockLogicalIndex; bi <= lastBlockLogicalIndex; ++bi) { - auto block = readBlock(list[bi]); - if (!block) { - kprintf("ext2fs: readInodeBytes: readBlock(%u) failed (lbi: %u)\n", list[bi], bi); - return -EIO; - } - - dword offsetIntoBlock; - - if (bi == firstBlockLogicalIndex) - offsetIntoBlock = offsetIntoFirstBlock; - else - offsetIntoBlock = 0; - - size_t numBytesToCopy = min(blockSize() - offsetIntoBlock, remainingCount); - memcpy(out, block.pointer() + offsetIntoBlock, numBytesToCopy); - remainingCount -= numBytesToCopy; - nread += numBytesToCopy; - out += numBytesToCopy; - } - - return nread; -} - bool Ext2FS::write_inode(InodeIdentifier inode, const ByteBuffer& data) { ASSERT(inode.fsid() == id()); diff --git a/VirtualFileSystem/Ext2FileSystem.h b/VirtualFileSystem/Ext2FileSystem.h index dec055c650..6499ec5088 100644 --- a/VirtualFileSystem/Ext2FileSystem.h +++ b/VirtualFileSystem/Ext2FileSystem.h @@ -74,7 +74,6 @@ private: virtual bool write_inode(InodeIdentifier, const ByteBuffer&) override; virtual InodeMetadata inode_metadata(InodeIdentifier) const override; virtual InodeIdentifier create_inode(InodeIdentifier parentInode, const String& name, Unix::mode_t, unsigned size, int& error) override; - virtual ssize_t read_inode_bytes(InodeIdentifier, Unix::off_t offset, size_t count, byte* buffer, FileDescriptor*) const override; virtual InodeIdentifier create_directory(InodeIdentifier parentInode, const String& name, Unix::mode_t, int& error) override; virtual InodeIdentifier find_parent_of_inode(InodeIdentifier) const override; virtual RetainPtr get_inode(InodeIdentifier) const override; diff --git a/VirtualFileSystem/FileDescriptor.cpp b/VirtualFileSystem/FileDescriptor.cpp index 4014a25310..8a9e9035f3 100644 --- a/VirtualFileSystem/FileDescriptor.cpp +++ b/VirtualFileSystem/FileDescriptor.cpp @@ -143,7 +143,8 @@ ssize_t FileDescriptor::read(byte* buffer, size_t count) // FIXME: What should happen to m_currentOffset? return m_vnode->characterDevice()->read(buffer, count); } - ssize_t nread = m_vnode->fs()->read_inode_bytes(m_vnode->inode, m_current_offset, count, buffer, this); + ASSERT(inode()); + ssize_t nread = inode()->read_bytes(m_current_offset, count, buffer, this); m_current_offset += nread; return nread; } diff --git a/VirtualFileSystem/FileSystem.cpp b/VirtualFileSystem/FileSystem.cpp index 0bb254096d..74c1d786b9 100644 --- a/VirtualFileSystem/FileSystem.cpp +++ b/VirtualFileSystem/FileSystem.cpp @@ -79,15 +79,16 @@ ByteBuffer Inode::read_entire(FileDescriptor* descriptor) */ } -ByteBuffer FS::read_entire_inode(InodeIdentifier inode, FileDescriptor* handle) const +ByteBuffer FS::read_entire_inode(InodeIdentifier inode_id, FileDescriptor* handle) const { - ASSERT(inode.fsid() == id()); + ASSERT(inode_id.fsid() == id()); - auto metadata = inode_metadata(inode); - if (!metadata.isValid()) { - kprintf("[fs] readInode: metadata lookup for inode %u failed\n", inode.index()); + auto inode = get_inode(inode_id); + if (!inode) { + kprintf("fs: read_entire_inode: lookup for inode %u:%u failed\n", id(), inode_id.index()); return nullptr; } + auto metadata = inode->metadata(); size_t initialSize = metadata.size ? metadata.size : 4096; auto contents = ByteBuffer::create_uninitialized(initialSize); @@ -97,7 +98,7 @@ ByteBuffer FS::read_entire_inode(InodeIdentifier inode, FileDescriptor* handle) byte* out = contents.pointer(); Unix::off_t offset = 0; for (;;) { - nread = read_inode_bytes(inode, offset, sizeof(buffer), buffer, handle); + nread = inode->read_bytes(offset, sizeof(buffer), buffer, handle); //kprintf("nread: %u, bufsiz: %u, initialSize: %u\n", nread, sizeof(buffer), initialSize); ASSERT(nread <= (ssize_t)sizeof(buffer)); if (nread <= 0) diff --git a/VirtualFileSystem/FileSystem.h b/VirtualFileSystem/FileSystem.h index 3f44a2b503..a6b86d8a9f 100644 --- a/VirtualFileSystem/FileSystem.h +++ b/VirtualFileSystem/FileSystem.h @@ -34,8 +34,6 @@ public: virtual bool write_inode(InodeIdentifier, const ByteBuffer&) = 0; virtual InodeMetadata inode_metadata(InodeIdentifier) const = 0; - virtual ssize_t read_inode_bytes(InodeIdentifier, Unix::off_t offset, size_t count, byte* buffer, FileDescriptor*) const = 0; - bool is_readonly() const { return m_readonly; } struct DirectoryEntry { diff --git a/VirtualFileSystem/SyntheticFileSystem.cpp b/VirtualFileSystem/SyntheticFileSystem.cpp index e4dde9e205..bc427d9926 100644 --- a/VirtualFileSystem/SyntheticFileSystem.cpp +++ b/VirtualFileSystem/SyntheticFileSystem.cpp @@ -163,43 +163,6 @@ bool SynthFS::write_inode(InodeIdentifier, const ByteBuffer&) return false; } -ssize_t SynthFS::read_inode_bytes(InodeIdentifier inode, Unix::off_t offset, size_t count, byte* buffer, FileDescriptor* handle) const -{ - ASSERT(inode.fsid() == id()); -#ifdef SYNTHFS_DEBUG - kprintf("SynthFS: readInode %u\n", inode.index()); -#endif - ASSERT(offset >= 0); - ASSERT(buffer); - - const SynthFSInode* found_file; - { - InterruptDisabler disabler; - auto it = m_inodes.find(inode.index()); - if (it == m_inodes.end()) - return false; - found_file = (*it).value.ptr(); - } - const SynthFSInode& file = *found_file; - ByteBuffer generatedData; - if (file.m_generator) { - if (!handle) { - generatedData = file.m_generator(); - } else { - if (!handle->generator_cache()) - handle->generator_cache() = file.m_generator(); - generatedData = handle->generator_cache(); - } - } - - auto* data = generatedData ? &generatedData : &file.m_data; - ssize_t nread = min(static_cast(data->size() - offset), static_cast(count)); - memcpy(buffer, data->pointer() + offset, nread); - if (nread == 0 && handle && handle->generator_cache()) - handle->generator_cache().clear(); - return nread; -} - InodeIdentifier SynthFS::create_directory(InodeIdentifier, const String&, Unix::mode_t, int& error) { error = -EROFS; diff --git a/VirtualFileSystem/SyntheticFileSystem.h b/VirtualFileSystem/SyntheticFileSystem.h index a40ab9977b..195a142af5 100644 --- a/VirtualFileSystem/SyntheticFileSystem.h +++ b/VirtualFileSystem/SyntheticFileSystem.h @@ -17,7 +17,6 @@ public: virtual bool write_inode(InodeIdentifier, const ByteBuffer&) override; virtual InodeMetadata inode_metadata(InodeIdentifier) const override; virtual InodeIdentifier create_inode(InodeIdentifier parentInode, const String& name, Unix::mode_t, unsigned size, int& error) override; - virtual ssize_t read_inode_bytes(InodeIdentifier, Unix::off_t offset, size_t count, byte* buffer, FileDescriptor*) const override; virtual InodeIdentifier create_directory(InodeIdentifier parentInode, const String& name, Unix::mode_t, int& error) override; virtual InodeIdentifier find_parent_of_inode(InodeIdentifier) const override; virtual RetainPtr get_inode(InodeIdentifier) const override; diff --git a/VirtualFileSystem/VirtualFileSystem.cpp b/VirtualFileSystem/VirtualFileSystem.cpp index 7ddba4aeb6..789ed67b61 100644 --- a/VirtualFileSystem/VirtualFileSystem.cpp +++ b/VirtualFileSystem/VirtualFileSystem.cpp @@ -391,15 +391,14 @@ InodeIdentifier VFS::resolve_path(const String& path, InodeIdentifier base, int& if (part.is_empty()) break; auto crumb_inode = get_inode(crumb_id); - ASSERT(crumb_inode); - auto metadata = crumb_inode->metadata(); - if (!metadata.isValid()) { + if (!crumb_inode) { #ifdef VFS_DEBUG kprintf("invalid metadata\n"); #endif error = -EIO; return { }; } + auto metadata = crumb_inode->metadata(); if (!metadata.isDirectory()) { #ifdef VFS_DEBUG kprintf("parent of <%s> not directory, it's inode %u:%u / %u:%u, mode: %u, size: %u\n", part.characters(), inode.fsid(), inode.index(), metadata.inode.fsid(), metadata.inode.index(), metadata.mode, metadata.size);