From eeaba41d130eb6c677a12c8618833990470c2cde Mon Sep 17 00:00:00 2001 From: Andreas Kling Date: Tue, 18 Aug 2020 12:41:27 +0200 Subject: [PATCH] Kernel: Add DirectoryEntryView for VFS directory traversal Unlike DirectoryEntry (which is used when constructing directories), DirectoryEntryView does not manage storage for file names. Names are just StringViews. This is much more suited to the directory traversal API and makes it easier to implement this in file system classes since they no longer need to create temporary name copies while traversing. --- Kernel/FileSystem/DevPtsFS.cpp | 8 +++---- Kernel/FileSystem/DevPtsFS.h | 2 +- Kernel/FileSystem/Ext2FileSystem.cpp | 10 ++++----- Kernel/FileSystem/Ext2FileSystem.h | 2 +- Kernel/FileSystem/FileDescription.cpp | 2 +- Kernel/FileSystem/FileSystem.cpp | 7 ++++++ Kernel/FileSystem/FileSystem.h | 9 ++++++++ Kernel/FileSystem/Inode.h | 2 +- Kernel/FileSystem/Plan9FileSystem.cpp | 16 +++---------- Kernel/FileSystem/Plan9FileSystem.h | 2 +- Kernel/FileSystem/ProcFS.cpp | 30 ++++++++++++------------- Kernel/FileSystem/ProcFS.h | 4 ++-- Kernel/FileSystem/TmpFS.cpp | 8 ++++--- Kernel/FileSystem/TmpFS.h | 2 +- Kernel/FileSystem/VirtualFileSystem.cpp | 8 +++---- Kernel/FileSystem/VirtualFileSystem.h | 2 +- 16 files changed, 61 insertions(+), 53 deletions(-) diff --git a/Kernel/FileSystem/DevPtsFS.cpp b/Kernel/FileSystem/DevPtsFS.cpp index 175c7075bf..ac060d026b 100644 --- a/Kernel/FileSystem/DevPtsFS.cpp +++ b/Kernel/FileSystem/DevPtsFS.cpp @@ -136,18 +136,18 @@ InodeMetadata DevPtsFSInode::metadata() const return m_metadata; } -KResult DevPtsFSInode::traverse_as_directory(Function callback) const +KResult DevPtsFSInode::traverse_as_directory(Function callback) const { if (identifier().index() > 1) return KResult(-ENOTDIR); - callback({ ".", 1, identifier(), 0 }); - callback({ "..", 2, identifier(), 0 }); + callback({ ".", identifier(), 0 }); + callback({ "..", identifier(), 0 }); for (unsigned pty_index : *ptys) { String name = String::number(pty_index); InodeIdentifier identifier = { fsid(), pty_index_to_inode_index(pty_index) }; - callback({ name.characters(), name.length(), identifier, 0 }); + callback({ name, identifier, 0 }); } return KSuccess; diff --git a/Kernel/FileSystem/DevPtsFS.h b/Kernel/FileSystem/DevPtsFS.h index 202a0a6d7d..1942a1f667 100644 --- a/Kernel/FileSystem/DevPtsFS.h +++ b/Kernel/FileSystem/DevPtsFS.h @@ -69,7 +69,7 @@ private: // ^Inode virtual ssize_t read_bytes(off_t, ssize_t, u8* buffer, FileDescription*) const override; virtual InodeMetadata metadata() const override; - virtual KResult traverse_as_directory(Function) const override; + virtual KResult traverse_as_directory(Function) const override; virtual RefPtr lookup(StringView name) override; virtual void flush_metadata() override; virtual ssize_t write_bytes(off_t, ssize_t, const u8* buffer, FileDescription*) override; diff --git a/Kernel/FileSystem/Ext2FileSystem.cpp b/Kernel/FileSystem/Ext2FileSystem.cpp index b537eb4df8..08e04e153a 100644 --- a/Kernel/FileSystem/Ext2FileSystem.cpp +++ b/Kernel/FileSystem/Ext2FileSystem.cpp @@ -849,7 +849,7 @@ ssize_t Ext2FSInode::write_bytes(off_t offset, ssize_t count, const u8* data, Fi return nwritten; } -KResult Ext2FSInode::traverse_as_directory(Function callback) const +KResult Ext2FSInode::traverse_as_directory(Function callback) const { LOCKER(m_lock); ASSERT(is_directory()); @@ -871,7 +871,7 @@ KResult Ext2FSInode::traverse_as_directory(Functioninode << ", name_len: " << entry->name_len << ", rec_len: " << entry->rec_len << ", file_type: " << entry->file_type << ", name: " << String(entry->name, entry->name_len); #endif - if (!callback({ entry->name, entry->name_len, { fsid(), entry->inode }, entry->file_type })) + if (!callback({ { entry->name, entry->name_len }, { fsid(), entry->inode }, entry->file_type })) break; } entry = (ext2_dir_entry_2*)((char*)entry + entry->rec_len); @@ -961,7 +961,7 @@ KResult Ext2FSInode::add_child(Inode& child, const StringView& name, mode_t mode name_already_exists = true; return false; } - entries.append(entry); + entries.append({ entry.name.characters_without_null_termination(), entry.name.length(), entry.inode, entry.file_type }); return true; }); @@ -1008,7 +1008,7 @@ KResult Ext2FSInode::remove_child(const StringView& name) Vector entries; KResult result = traverse_as_directory([&](auto& entry) { if (name != entry.name) - entries.append(entry); + entries.append({ entry.name.characters_without_null_termination(), entry.name.length(), entry.inode, entry.file_type }); return true; }); if (result.is_error()) @@ -1478,7 +1478,7 @@ void Ext2FSInode::populate_lookup_cache() const HashMap children; KResult result = traverse_as_directory([&children](auto& entry) { - children.set(String(entry.name, entry.name_length), entry.inode.index()); + children.set(entry.name, entry.inode.index()); return true; }); diff --git a/Kernel/FileSystem/Ext2FileSystem.h b/Kernel/FileSystem/Ext2FileSystem.h index db3ddaaac5..5790e380c4 100644 --- a/Kernel/FileSystem/Ext2FileSystem.h +++ b/Kernel/FileSystem/Ext2FileSystem.h @@ -59,7 +59,7 @@ private: // ^Inode virtual ssize_t read_bytes(off_t, ssize_t, u8* buffer, FileDescription*) const override; virtual InodeMetadata metadata() const override; - virtual KResult traverse_as_directory(Function) const override; + virtual KResult traverse_as_directory(Function) const override; virtual RefPtr lookup(StringView name) override; virtual void flush_metadata() override; virtual ssize_t write_bytes(off_t, ssize_t, const u8* data, FileDescription*) override; diff --git a/Kernel/FileSystem/FileDescription.cpp b/Kernel/FileSystem/FileDescription.cpp index dff77e36ea..ce0ddd0715 100644 --- a/Kernel/FileSystem/FileDescription.cpp +++ b/Kernel/FileSystem/FileDescription.cpp @@ -191,7 +191,7 @@ ssize_t FileDescription::get_dir_entries(u8* buffer, ssize_t size) KResult result = VFS::the().traverse_directory_inode(*m_inode, [&stream](auto& entry) { stream << (u32)entry.inode.index(); stream << (u8)entry.file_type; - stream << (u32)entry.name_length; + stream << (u32)entry.name.length(); stream << entry.name; return true; }); diff --git a/Kernel/FileSystem/FileSystem.cpp b/Kernel/FileSystem/FileSystem.cpp index 817bf09c6b..20952c2deb 100644 --- a/Kernel/FileSystem/FileSystem.cpp +++ b/Kernel/FileSystem/FileSystem.cpp @@ -85,6 +85,13 @@ FS::DirectoryEntry::DirectoryEntry(const char* n, size_t nl, InodeIdentifier i, name[nl] = '\0'; } +FS::DirectoryEntryView::DirectoryEntryView(const StringView& n, InodeIdentifier i, u8 ft) + : name(n) + , inode(i) + , file_type(ft) +{ +} + void FS::sync() { Inode::sync(); diff --git a/Kernel/FileSystem/FileSystem.h b/Kernel/FileSystem/FileSystem.h index 8a7f3581da..6e634c14b9 100644 --- a/Kernel/FileSystem/FileSystem.h +++ b/Kernel/FileSystem/FileSystem.h @@ -29,6 +29,7 @@ #include #include #include +#include #include #include #include @@ -78,6 +79,14 @@ public: u8 file_type { 0 }; }; + struct DirectoryEntryView { + DirectoryEntryView(const StringView& name, InodeIdentifier, u8 file_type); + + StringView name; + InodeIdentifier inode; + u8 file_type { 0 }; + }; + virtual void flush_writes() { } size_t block_size() const { return m_block_size; } diff --git a/Kernel/FileSystem/Inode.h b/Kernel/FileSystem/Inode.h index 24e7f12012..f9ed0db158 100644 --- a/Kernel/FileSystem/Inode.h +++ b/Kernel/FileSystem/Inode.h @@ -70,7 +70,7 @@ public: KResultOr read_entire(FileDescription* = nullptr) const; virtual ssize_t read_bytes(off_t, ssize_t, u8* buffer, FileDescription*) const = 0; - virtual KResult traverse_as_directory(Function) const = 0; + virtual KResult traverse_as_directory(Function) const = 0; virtual RefPtr lookup(StringView name) = 0; virtual ssize_t write_bytes(off_t, ssize_t, const u8* data, FileDescription*) = 0; virtual KResultOr> create_child(const String& name, mode_t, dev_t, uid_t, gid_t) = 0; diff --git a/Kernel/FileSystem/Plan9FileSystem.cpp b/Kernel/FileSystem/Plan9FileSystem.cpp index 816ba650a7..952a176066 100644 --- a/Kernel/FileSystem/Plan9FileSystem.cpp +++ b/Kernel/FileSystem/Plan9FileSystem.cpp @@ -798,7 +798,7 @@ void Plan9FSInode::flush_metadata() KResultOr Plan9FSInode::directory_entry_count() const { size_t count = 0; - KResult result = traverse_as_directory([&count](const FS::DirectoryEntry&) { + KResult result = traverse_as_directory([&count](auto&) { count++; return true; }); @@ -809,7 +809,7 @@ KResultOr Plan9FSInode::directory_entry_count() const return count; } -KResult Plan9FSInode::traverse_as_directory(Function callback) const +KResult Plan9FSInode::traverse_as_directory(Function callback) const { KResult result = KSuccess; @@ -857,17 +857,7 @@ KResult Plan9FSInode::traverse_as_directory(Function> qid >> offset >> type >> name; - - FS::DirectoryEntry entry { - "", - name.length(), - { fsid(), fs().allocate_fid() }, - 0 - }; - size_t size_to_copy = min(sizeof(entry.name) - 1, name.length()); - memcpy(entry.name, name.characters_without_null_termination(), size_to_copy); - entry.name[size_to_copy] = 0; - callback(entry); + callback({ name, { fsid(), fs().allocate_fid() }, 0 }); } } diff --git a/Kernel/FileSystem/Plan9FileSystem.h b/Kernel/FileSystem/Plan9FileSystem.h index cb4ac9adc1..77af39e47a 100644 --- a/Kernel/FileSystem/Plan9FileSystem.h +++ b/Kernel/FileSystem/Plan9FileSystem.h @@ -126,7 +126,7 @@ public: virtual void flush_metadata() override; virtual ssize_t read_bytes(off_t, ssize_t, u8* buffer, FileDescription*) const override; virtual ssize_t write_bytes(off_t, ssize_t, const u8* data, FileDescription*) override; - virtual KResult traverse_as_directory(Function) const override; + virtual KResult traverse_as_directory(Function) const override; virtual RefPtr lookup(StringView name) override; virtual KResultOr> create_child(const String& name, mode_t, dev_t, uid_t, gid_t) override; virtual KResult add_child(Inode&, const StringView& name, mode_t) override; diff --git a/Kernel/FileSystem/ProcFS.cpp b/Kernel/FileSystem/ProcFS.cpp index 3aaf589cca..b7d306e17d 100644 --- a/Kernel/FileSystem/ProcFS.cpp +++ b/Kernel/FileSystem/ProcFS.cpp @@ -1246,7 +1246,7 @@ InodeIdentifier ProcFS::ProcFSDirectoryEntry::identifier(unsigned fsid) const return to_identifier(fsid, PDI_Root, 0, (ProcFileType)proc_file_type); } -KResult ProcFSInode::traverse_as_directory(Function callback) const +KResult ProcFSInode::traverse_as_directory(Function callback) const { #ifdef PROCFS_DEBUG dbg() << "ProcFS: traverse_as_directory " << index(); @@ -1258,8 +1258,8 @@ KResult ProcFSInode::traverse_as_directory(Function __FI_Root_Start && entry.proc_file_type < __FI_Root_End) - callback({ entry.name, strlen(entry.name), to_identifier(fsid(), PDI_Root, 0, (ProcFileType)entry.proc_file_type), 0 }); + callback({ { entry.name, strlen(entry.name) }, to_identifier(fsid(), PDI_Root, 0, (ProcFileType)entry.proc_file_type), 0 }); } for (auto pid_child : Process::all_pids()) { char name[16]; size_t name_length = (size_t)sprintf(name, "%d", pid_child.value()); - callback({ name, name_length, to_identifier(fsid(), PDI_Root, pid_child, FI_PID), 0 }); + callback({ { name, name_length }, to_identifier(fsid(), PDI_Root, pid_child, FI_PID), 0 }); } break; case FI_Root_sys: for (size_t i = 1; i < sys_variables().size(); ++i) { auto& variable = sys_variables()[i]; - callback({ variable.name.characters(), variable.name.length(), sys_var_to_identifier(fsid(), i), 0 }); + callback({ variable.name, sys_var_to_identifier(fsid(), i), 0 }); } break; case FI_Root_net: - callback({ "adapters", 8, to_identifier(fsid(), PDI_Root_net, 0, FI_Root_net_adapters), 0 }); - callback({ "arp", 3, to_identifier(fsid(), PDI_Root_net, 0, FI_Root_net_arp), 0 }); - callback({ "tcp", 3, to_identifier(fsid(), PDI_Root_net, 0, FI_Root_net_tcp), 0 }); - callback({ "udp", 3, to_identifier(fsid(), PDI_Root_net, 0, FI_Root_net_udp), 0 }); - callback({ "local", 5, to_identifier(fsid(), PDI_Root_net, 0, FI_Root_net_local), 0 }); + callback({ "adapters", to_identifier(fsid(), PDI_Root_net, 0, FI_Root_net_adapters), 0 }); + callback({ "arp", to_identifier(fsid(), PDI_Root_net, 0, FI_Root_net_arp), 0 }); + callback({ "tcp", to_identifier(fsid(), PDI_Root_net, 0, FI_Root_net_tcp), 0 }); + callback({ "udp", to_identifier(fsid(), PDI_Root_net, 0, FI_Root_net_udp), 0 }); + callback({ "local", to_identifier(fsid(), PDI_Root_net, 0, FI_Root_net_local), 0 }); break; case FI_PID: { @@ -1302,7 +1302,7 @@ KResult ProcFSInode::traverse_as_directory(Functionexecutable()) continue; // FIXME: strlen() here is sad. - callback({ entry.name, strlen(entry.name), to_identifier(fsid(), PDI_PID, pid, (ProcFileType)entry.proc_file_type), 0 }); + callback({ { entry.name, strlen(entry.name) }, to_identifier(fsid(), PDI_PID, pid, (ProcFileType)entry.proc_file_type), 0 }); } } } break; @@ -1318,7 +1318,7 @@ KResult ProcFSInode::traverse_as_directory(Function ProcFSInode::directory_entry_count() const { ASSERT(is_directory()); size_t count = 0; - KResult result = traverse_as_directory([&count](const FS::DirectoryEntry&) { + KResult result = traverse_as_directory([&count](auto&) { ++count; return true; }); diff --git a/Kernel/FileSystem/ProcFS.h b/Kernel/FileSystem/ProcFS.h index 4d79096b51..a68df543f0 100644 --- a/Kernel/FileSystem/ProcFS.h +++ b/Kernel/FileSystem/ProcFS.h @@ -98,7 +98,7 @@ private: // ^Inode virtual ssize_t read_bytes(off_t, ssize_t, u8* buffer, FileDescription*) const override; virtual InodeMetadata metadata() const override; - virtual KResult traverse_as_directory(Function) const override; + virtual KResult traverse_as_directory(Function) const override; virtual RefPtr lookup(StringView name) override; virtual void flush_metadata() override; virtual ssize_t write_bytes(off_t, ssize_t, const u8* buffer, FileDescription*) override; @@ -125,7 +125,7 @@ private: // ^Inode virtual ssize_t read_bytes(off_t, ssize_t, u8*, FileDescription*) const override { ASSERT_NOT_REACHED(); } virtual InodeMetadata metadata() const override; - virtual KResult traverse_as_directory(Function) const override { ASSERT_NOT_REACHED(); } + virtual KResult traverse_as_directory(Function) const override { ASSERT_NOT_REACHED(); } virtual RefPtr lookup(StringView name) override; virtual void flush_metadata() override {}; virtual ssize_t write_bytes(off_t, ssize_t, const u8*, FileDescription*) override { ASSERT_NOT_REACHED(); } diff --git a/Kernel/FileSystem/TmpFS.cpp b/Kernel/FileSystem/TmpFS.cpp index cb72bfc537..c505c1cf39 100644 --- a/Kernel/FileSystem/TmpFS.cpp +++ b/Kernel/FileSystem/TmpFS.cpp @@ -124,7 +124,7 @@ InodeMetadata TmpFSInode::metadata() const return m_metadata; } -KResult TmpFSInode::traverse_as_directory(Function callback) const +KResult TmpFSInode::traverse_as_directory(Function callback) const { LOCKER(m_lock, Lock::Mode::Shared); @@ -134,8 +134,10 @@ KResult TmpFSInode::traverse_as_directory(Function) const override; + virtual KResult traverse_as_directory(Function) const override; virtual RefPtr lookup(StringView name) override; virtual void flush_metadata() override; virtual ssize_t write_bytes(off_t, ssize_t, const u8* buffer, FileDescription*) override; diff --git a/Kernel/FileSystem/VirtualFileSystem.cpp b/Kernel/FileSystem/VirtualFileSystem.cpp index 3a1bf93a1e..5febd7c5e6 100644 --- a/Kernel/FileSystem/VirtualFileSystem.cpp +++ b/Kernel/FileSystem/VirtualFileSystem.cpp @@ -191,9 +191,9 @@ bool VFS::is_vfs_root(InodeIdentifier inode) const return inode == root_inode_id(); } -KResult VFS::traverse_directory_inode(Inode& dir_inode, Function callback) +KResult VFS::traverse_directory_inode(Inode& dir_inode, Function callback) { - return dir_inode.traverse_as_directory([&](const FS::DirectoryEntry& entry) { + return dir_inode.traverse_as_directory([&](auto& entry) { InodeIdentifier resolved_inode; if (auto mount = find_mount_for_host(entry.inode)) resolved_inode = mount->guest().identifier(); @@ -202,13 +202,13 @@ KResult VFS::traverse_directory_inode(Inode& dir_inode, Functionidentifier(); - if (is_root_inode && !is_vfs_root(dir_inode.identifier()) && !strcmp(entry.name, "..")) { + if (is_root_inode && !is_vfs_root(dir_inode.identifier()) && entry.name == "..") { auto mount = find_mount_for_guest(dir_inode); ASSERT(mount); ASSERT(mount->host()); resolved_inode = mount->host()->identifier(); } - callback(FS::DirectoryEntry(entry.name, entry.name_length, resolved_inode, entry.file_type)); + callback({ entry.name, resolved_inode, entry.file_type }); return true; }); } diff --git a/Kernel/FileSystem/VirtualFileSystem.h b/Kernel/FileSystem/VirtualFileSystem.h index 923437c8c2..35b17e0da8 100644 --- a/Kernel/FileSystem/VirtualFileSystem.h +++ b/Kernel/FileSystem/VirtualFileSystem.h @@ -126,7 +126,7 @@ private: bool is_vfs_root(InodeIdentifier) const; - KResult traverse_directory_inode(Inode&, Function); + KResult traverse_directory_inode(Inode&, Function); Mount* find_mount_for_host(Inode&); Mount* find_mount_for_host(InodeIdentifier);