Kernel: Plumb KResult through FileDescription::read_entire_file() implementation.

Allow file system implementation to return meaningful error codes to
callers of the FileDescription::read_entire_file(). This allows both
Process::sys$readlink() and Process::sys$module_load() to return more
detailed errors to the user.

12 files changed, 44 insertions, 30 deletions

diff --git a/Kernel/FileSystem/DevPtsFS.cpp b/Kernel/FileSystem/DevPtsFS.cpp
index 5bb0266b6b..cf0237584e 100644
--- a/Kernel/FileSystem/DevPtsFS.cpp
+++ b/Kernel/FileSystem/DevPtsFS.cpp
@@ -146,10 +146,10 @@ InodeMetadata DevPtsFSInode::metadata() const
     return m_metadata;
 }
 
-bool DevPtsFSInode::traverse_as_directory(Function<bool(const FS::DirectoryEntry&)> callback) const
+KResult DevPtsFSInode::traverse_as_directory(Function<bool(const FS::DirectoryEntry&)> callback) const
 {
     if (identifier().index() > 1)
-        return false;
+        return KResult(-ENOTDIR);
 
     callback({ ".", 1, identifier(), 0 });
     callback({ "..", 2, identifier(), 0 });
@@ -160,7 +160,7 @@ bool DevPtsFSInode::traverse_as_directory(Function<bool(const FS::DirectoryEntry
         callback({ name.characters(), name.length(), identifier, 0 });
     }
 
-    return true;
+    return KSuccess;
 }
 
 size_t DevPtsFSInode::directory_entry_count() const
diff --git a/Kernel/FileSystem/DevPtsFS.h b/Kernel/FileSystem/DevPtsFS.h
index 7424d40156..2b97f105d7 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 bool traverse_as_directory(Function<bool(const FS::DirectoryEntry&)>) const override;
+    virtual KResult traverse_as_directory(Function<bool(const FS::DirectoryEntry&)>) const override;
     virtual RefPtr<Inode> 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 3d15225b2a..e1533fc3a6 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;
 }
 
-bool Ext2FSInode::traverse_as_directory(Function<bool(const FS::DirectoryEntry&)> callback) const
+KResult Ext2FSInode::traverse_as_directory(Function<bool(const FS::DirectoryEntry&)> callback) const
 {
     LOCKER(m_lock);
     ASSERT(is_directory());
@@ -858,8 +858,12 @@ bool Ext2FSInode::traverse_as_directory(Function<bool(const FS::DirectoryEntry&)
     dbg() << "Ext2FS: Traversing as directory: " << identifier();
 #endif
 
-    auto buffer = read_entire();
-    ASSERT(buffer);
+    auto buffer_or = read_entire();
+    ASSERT(!buffer_or.is_error());
+    if (buffer_or.is_error())
+        return buffer_or.error();
+
+    auto buffer = buffer_or.value();
     auto* entry = reinterpret_cast<ext2_dir_entry_2*>(buffer.data());
 
     while (entry < buffer.end_pointer()) {
@@ -872,7 +876,8 @@ bool Ext2FSInode::traverse_as_directory(Function<bool(const FS::DirectoryEntry&)
         }
         entry = (ext2_dir_entry_2*)((char*)entry + entry->rec_len);
     }
-    return true;
+
+    return KSuccess;
 }
 
 bool Ext2FSInode::write_directory(const Vector<FS::DirectoryEntry>& entries)
@@ -944,7 +949,7 @@ KResult Ext2FSInode::add_child(InodeIdentifier child_id, const StringView& name,
 
     Vector<FS::DirectoryEntry> entries;
     bool name_already_exists = false;
-    traverse_as_directory([&](auto& entry) {
+    KResult result = traverse_as_directory([&](auto& entry) {
         if (name == entry.name) {
             name_already_exists = true;
             return false;
@@ -952,6 +957,10 @@ KResult Ext2FSInode::add_child(InodeIdentifier child_id, const StringView& name,
         entries.append(entry);
         return true;
     });
+
+    if (result.is_error())
+        return result;
+
     if (name_already_exists) {
         dbg() << "Ext2FSInode::add_child(): Name '" << name << "' already exists in inode " << index();
         return KResult(-EEXIST);
@@ -959,7 +968,7 @@ KResult Ext2FSInode::add_child(InodeIdentifier child_id, const StringView& name,
 
     auto child_inode = fs().get_inode(child_id);
     if (child_inode) {
-        auto result = child_inode->increment_link_count();
+        result = child_inode->increment_link_count();
         if (result.is_error())
             return result;
     }
@@ -991,11 +1000,13 @@ KResult Ext2FSInode::remove_child(const StringView& name)
 #endif
 
     Vector<FS::DirectoryEntry> entries;
-    traverse_as_directory([&](auto& entry) {
+    KResult result = traverse_as_directory([&](auto& entry) {
         if (name != entry.name)
             entries.append(entry);
         return true;
     });
+    if (result.is_error())
+        return result;
 
     bool success = write_directory(entries);
     if (!success) {
diff --git a/Kernel/FileSystem/Ext2FileSystem.h b/Kernel/FileSystem/Ext2FileSystem.h
index 5c75ec8667..1862a286b2 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 bool traverse_as_directory(Function<bool(const FS::DirectoryEntry&)>) const override;
+    virtual KResult traverse_as_directory(Function<bool(const FS::DirectoryEntry&)>) const override;
     virtual RefPtr<Inode> 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 d504032566..6b7e4a507a 100644
--- a/Kernel/FileSystem/FileDescription.cpp
+++ b/Kernel/FileSystem/FileDescription.cpp
@@ -158,7 +158,7 @@ bool FileDescription::can_read() const
     return m_file->can_read(*this, offset());
 }
 
-ByteBuffer FileDescription::read_entire_file()
+KResultOr<ByteBuffer> FileDescription::read_entire_file()
 {
     // HACK ALERT: (This entire function)
     ASSERT(m_file->is_inode());
diff --git a/Kernel/FileSystem/FileDescription.h b/Kernel/FileSystem/FileDescription.h
index d0a3c7e998..f3b8108083 100644
--- a/Kernel/FileSystem/FileDescription.h
+++ b/Kernel/FileSystem/FileDescription.h
@@ -79,7 +79,7 @@ public:
 
     ssize_t get_dir_entries(u8* buffer, ssize_t);
 
-    ByteBuffer read_entire_file();
+    KResultOr<ByteBuffer> read_entire_file();
 
     String absolute_path() const;
 
diff --git a/Kernel/FileSystem/Inode.cpp b/Kernel/FileSystem/Inode.cpp
index bc4d3c2331..8befec5363 100644
--- a/Kernel/FileSystem/Inode.cpp
+++ b/Kernel/FileSystem/Inode.cpp
@@ -61,7 +61,7 @@ void Inode::sync()
     }
 }
 
-ByteBuffer Inode::read_entire(FileDescription* descriptor) const
+KResultOr<ByteBuffer> Inode::read_entire(FileDescription* descriptor) const
 {
     size_t initial_size = metadata().size ? metadata().size : 4096;
     StringBuilder builder(initial_size);
@@ -92,8 +92,11 @@ KResultOr<NonnullRefPtr<Custody>> Inode::resolve_as_link(Custody& base, RefPtr<C
     // The default implementation simply treats the stored
     // contents as a path and resolves that. That is, it
     // behaves exactly how you would expect a symlink to work.
-    auto contents = read_entire();
+    auto contents_or = read_entire();
+    if (contents_or.is_error())
+        return contents_or.error();
 
+    auto& contents = contents_or.value();
     if (!contents) {
         if (out_parent)
             *out_parent = nullptr;
diff --git a/Kernel/FileSystem/Inode.h b/Kernel/FileSystem/Inode.h
index 33716e4ff9..b34f2cb208 100644
--- a/Kernel/FileSystem/Inode.h
+++ b/Kernel/FileSystem/Inode.h
@@ -66,10 +66,10 @@ public:
     InodeIdentifier identifier() const { return { fsid(), index() }; }
     virtual InodeMetadata metadata() const = 0;
 
-    ByteBuffer read_entire(FileDescription* = nullptr) const;
+    KResultOr<ByteBuffer> read_entire(FileDescription* = nullptr) const;
 
     virtual ssize_t read_bytes(off_t, ssize_t, u8* buffer, FileDescription*) const = 0;
-    virtual bool traverse_as_directory(Function<bool(const FS::DirectoryEntry&)>) const = 0;
+    virtual KResult traverse_as_directory(Function<bool(const FS::DirectoryEntry&)>) const = 0;
     virtual RefPtr<Inode> lookup(StringView name) = 0;
     virtual ssize_t write_bytes(off_t, ssize_t, const u8* data, FileDescription*) = 0;
     virtual KResult add_child(InodeIdentifier child_id, const StringView& name, mode_t) = 0;
diff --git a/Kernel/FileSystem/ProcFS.cpp b/Kernel/FileSystem/ProcFS.cpp
index ce5226f861..659dd19fc9 100644
--- a/Kernel/FileSystem/ProcFS.cpp
+++ b/Kernel/FileSystem/ProcFS.cpp
@@ -1253,14 +1253,14 @@ InodeIdentifier ProcFS::ProcFSDirectoryEntry::identifier(unsigned fsid) const
     return to_identifier(fsid, PDI_Root, 0, (ProcFileType)proc_file_type);
 }
 
-bool ProcFSInode::traverse_as_directory(Function<bool(const FS::DirectoryEntry&)> callback) const
+KResult ProcFSInode::traverse_as_directory(Function<bool(const FS::DirectoryEntry&)> callback) const
 {
 #ifdef PROCFS_DEBUG
     dbg() << "ProcFS: traverse_as_directory " << index();
 #endif
 
     if (!Kernel::is_directory(identifier()))
-        return false;
+        return KResult(-ENOTDIR);
 
     auto pid = to_pid(identifier());
     auto proc_file_type = to_proc_file_type(identifier());
@@ -1303,7 +1303,7 @@ bool ProcFSInode::traverse_as_directory(Function<bool(const FS::DirectoryEntry&)
     case FI_PID: {
         auto handle = ProcessInspectionHandle::from_pid(pid);
         if (!handle)
-            return false;
+            return KResult(-ENOENT);
         auto& process = handle->process();
         for (auto& entry : fs().m_entries) {
             if (entry.proc_file_type > __FI_PID_Start && entry.proc_file_type < __FI_PID_End) {
@@ -1318,7 +1318,7 @@ bool ProcFSInode::traverse_as_directory(Function<bool(const FS::DirectoryEntry&)
     case FI_PID_fd: {
         auto handle = ProcessInspectionHandle::from_pid(pid);
         if (!handle)
-            return false;
+            return KResult(-ENOENT);
         auto& process = handle->process();
         for (int i = 0; i < process.max_open_file_descriptors(); ++i) {
             auto description = process.file_description(i);
@@ -1330,10 +1330,10 @@ bool ProcFSInode::traverse_as_directory(Function<bool(const FS::DirectoryEntry&)
         }
     } break;
     default:
-        return true;
+        return KSuccess;
     }
 
-    return true;
+    return KSuccess;
 }
 
 RefPtr<Inode> ProcFSInode::lookup(StringView name)
diff --git a/Kernel/FileSystem/ProcFS.h b/Kernel/FileSystem/ProcFS.h
index 978b704018..d498c27b0a 100644
--- a/Kernel/FileSystem/ProcFS.h
+++ b/Kernel/FileSystem/ProcFS.h
@@ -101,7 +101,7 @@ private:
     // ^Inode
     virtual ssize_t read_bytes(off_t, ssize_t, u8* buffer, FileDescription*) const override;
     virtual InodeMetadata metadata() const override;
-    virtual bool traverse_as_directory(Function<bool(const FS::DirectoryEntry&)>) const override;
+    virtual KResult traverse_as_directory(Function<bool(const FS::DirectoryEntry&)>) const override;
     virtual RefPtr<Inode> lookup(StringView name) override;
     virtual void flush_metadata() override;
     virtual ssize_t write_bytes(off_t, ssize_t, const u8* buffer, FileDescription*) override;
@@ -127,7 +127,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 bool traverse_as_directory(Function<bool(const FS::DirectoryEntry&)>) const override { ASSERT_NOT_REACHED(); }
+    virtual KResult traverse_as_directory(Function<bool(const FS::DirectoryEntry&)>) const override { ASSERT_NOT_REACHED(); }
     virtual RefPtr<Inode> 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 74b5980da8..a0cd624137 100644
--- a/Kernel/FileSystem/TmpFS.cpp
+++ b/Kernel/FileSystem/TmpFS.cpp
@@ -166,19 +166,19 @@ InodeMetadata TmpFSInode::metadata() const
     return m_metadata;
 }
 
-bool TmpFSInode::traverse_as_directory(Function<bool(const FS::DirectoryEntry&)> callback) const
+KResult TmpFSInode::traverse_as_directory(Function<bool(const FS::DirectoryEntry&)> callback) const
 {
     LOCKER(m_lock, Lock::Mode::Shared);
 
     if (!is_directory())
-        return false;
+        return KResult(-ENOTDIR);
 
     callback({ ".", identifier(), 0 });
     callback({ "..", m_parent, 0 });
 
     for (auto& it : m_children)
         callback(it.value.entry);
-    return true;
+    return KSuccess;
 }
 
 ssize_t TmpFSInode::read_bytes(off_t offset, ssize_t size, u8* buffer, FileDescription*) const
diff --git a/Kernel/FileSystem/TmpFS.h b/Kernel/FileSystem/TmpFS.h
index 65c910ba13..b90aac1f47 100644
--- a/Kernel/FileSystem/TmpFS.h
+++ b/Kernel/FileSystem/TmpFS.h
@@ -79,7 +79,7 @@ public:
     // ^Inode
     virtual ssize_t read_bytes(off_t, ssize_t, u8* buffer, FileDescription*) const override;
     virtual InodeMetadata metadata() const override;
-    virtual bool traverse_as_directory(Function<bool(const FS::DirectoryEntry&)>) const override;
+    virtual KResult traverse_as_directory(Function<bool(const FS::DirectoryEntry&)>) const override;
     virtual RefPtr<Inode> lookup(StringView name) override;
     virtual void flush_metadata() override;
     virtual ssize_t write_bytes(off_t, ssize_t, const u8* buffer, FileDescription*) override;