Kernel: Make FileSystem::get_inode() return KResultOr<NRP<Inode>>

This allows for natural error propagation in a bunch of new places.

8 files changed, 33 insertions, 49 deletions

diff --git a/Kernel/FileSystem/DevFS.cpp b/Kernel/FileSystem/DevFS.cpp
index 35316f663b..1571cb3d2e 100644
--- a/Kernel/FileSystem/DevFS.cpp
+++ b/Kernel/FileSystem/DevFS.cpp
@@ -65,7 +65,7 @@ Inode& DevFS::root_inode()
     return *m_root_inode;
 }
 
-RefPtr<Inode> DevFS::get_inode(InodeIdentifier inode_id) const
+KResultOr<NonnullRefPtr<Inode>> DevFS::get_inode(InodeIdentifier inode_id) const
 {
     MutexLocker locker(m_lock);
     if (inode_id.index() == 1)
@@ -74,7 +74,7 @@ RefPtr<Inode> DevFS::get_inode(InodeIdentifier inode_id) const
         if (inode_id.index() == node.index())
             return node;
     }
-    return nullptr;
+    return ENOENT;
 }
 
 DevFSInode::DevFSInode(DevFS& fs)
diff --git a/Kernel/FileSystem/DevFS.h b/Kernel/FileSystem/DevFS.h
index fb0a64ae1d..bbb5ed498d 100644
--- a/Kernel/FileSystem/DevFS.h
+++ b/Kernel/FileSystem/DevFS.h
@@ -32,7 +32,7 @@ public:
 
 private:
     DevFS();
-    RefPtr<Inode> get_inode(InodeIdentifier) const;
+    KResultOr<NonnullRefPtr<Inode>> get_inode(InodeIdentifier) const;
     size_t allocate_inode_index();
 
     NonnullRefPtr<DevFSRootDirectoryInode> m_root_inode;
diff --git a/Kernel/FileSystem/DevPtsFS.cpp b/Kernel/FileSystem/DevPtsFS.cpp
index 555b2cbc88..30f0fee88f 100644
--- a/Kernel/FileSystem/DevPtsFS.cpp
+++ b/Kernel/FileSystem/DevPtsFS.cpp
@@ -55,15 +55,16 @@ Inode& DevPtsFS::root_inode()
     return *m_root_inode;
 }
 
-RefPtr<Inode> DevPtsFS::get_inode(InodeIdentifier inode_id) const
+KResultOr<NonnullRefPtr<Inode>> DevPtsFS::get_inode(InodeIdentifier inode_id) const
 {
     if (inode_id.index() == 1)
-        return m_root_inode;
+        return *m_root_inode;
 
     unsigned pty_index = inode_index_to_pty_index(inode_id.index());
     auto* device = Device::get_device(201, pty_index);
     VERIFY(device);
 
+    // FIXME: Handle OOM
     auto inode = adopt_ref(*new DevPtsFSInode(const_cast<DevPtsFS&>(*this), inode_id.index(), static_cast<SlavePTY*>(device)));
     inode->m_metadata.inode = inode_id;
     inode->m_metadata.size = 0;
@@ -142,10 +143,7 @@ KResultOr<NonnullRefPtr<Inode>> DevPtsFSInode::lookup(StringView name)
         for (SlavePTY& slave_pty : list) {
             if (slave_pty.index() != pty_index.value())
                 continue;
-            auto inode = fs().get_inode({ fsid(), pty_index_to_inode_index(pty_index.value()) });
-            if (!inode)
-                return ENOENT;
-            return inode.release_nonnull();
+            return fs().get_inode({ fsid(), pty_index_to_inode_index(pty_index.value()) });
         }
         return ENOENT;
     });
diff --git a/Kernel/FileSystem/DevPtsFS.h b/Kernel/FileSystem/DevPtsFS.h
index 5e82671274..801e42f699 100644
--- a/Kernel/FileSystem/DevPtsFS.h
+++ b/Kernel/FileSystem/DevPtsFS.h
@@ -29,7 +29,7 @@ public:
 
 private:
     DevPtsFS();
-    RefPtr<Inode> get_inode(InodeIdentifier) const;
+    KResultOr<NonnullRefPtr<Inode>> get_inode(InodeIdentifier) const;
 
     RefPtr<DevPtsFSInode> m_root_inode;
 };
diff --git a/Kernel/FileSystem/Ext2FileSystem.cpp b/Kernel/FileSystem/Ext2FileSystem.cpp
index 869cad637f..2033bbc455 100644
--- a/Kernel/FileSystem/Ext2FileSystem.cpp
+++ b/Kernel/FileSystem/Ext2FileSystem.cpp
@@ -147,12 +147,7 @@ KResult Ext2FS::initialize()
         }
     }
 
-    m_root_inode = static_ptr_cast<Ext2FSInode>(get_inode({ fsid(), EXT2_ROOT_INO }));
-    if (!m_root_inode) {
-        dbgln("Ext2FS: failed to acquire root inode");
-        return EINVAL;
-    }
-
+    m_root_inode = static_ptr_cast<Ext2FSInode>(TRY(get_inode({ fsid(), EXT2_ROOT_INO })));
     return KSuccess;
 }
 
@@ -781,37 +776,37 @@ void Ext2FSInode::flush_metadata()
     set_metadata_dirty(false);
 }
 
-RefPtr<Inode> Ext2FS::get_inode(InodeIdentifier inode) const
+KResultOr<NonnullRefPtr<Inode>> Ext2FS::get_inode(InodeIdentifier inode) const
 {
     MutexLocker locker(m_lock);
     VERIFY(inode.fsid() == fsid());
 
     {
         auto it = m_inode_cache.find(inode.index());
-        if (it != m_inode_cache.end())
-            return (*it).value;
+        if (it != m_inode_cache.end()) {
+            if (!it->value)
+                return ENOENT;
+            return *it->value;
+        }
     }
 
-    auto state_or_error = get_inode_allocation_state(inode.index());
-    if (state_or_error.is_error())
-        return {};
+    auto inode_allocation_state = TRY(get_inode_allocation_state(inode.index()));
 
-    if (!state_or_error.value()) {
+    if (!inode_allocation_state) {
         m_inode_cache.set(inode.index(), nullptr);
-        return {};
+        return ENOENT;
     }
 
     BlockIndex block_index;
     unsigned offset;
     if (!find_block_containing_inode(inode.index(), block_index, offset))
-        return {};
+        return EINVAL;
+
+    auto new_inode = TRY(adopt_nonnull_ref_or_enomem(new (nothrow) Ext2FSInode(const_cast<Ext2FS&>(*this), inode.index())));
 
-    auto new_inode = adopt_ref(*new Ext2FSInode(const_cast<Ext2FS&>(*this), inode.index()));
     auto buffer = UserOrKernelBuffer::for_kernel_buffer(reinterpret_cast<u8*>(&new_inode->m_raw_inode));
-    if (auto result = read_block(block_index, &buffer, sizeof(ext2_inode), offset); result.is_error()) {
-        // FIXME: Propagate the actual error.
-        return nullptr;
-    }
+    TRY(read_block(block_index, &buffer, sizeof(ext2_inode), offset));
+
     m_inode_cache.set(inode.index(), new_inode);
     return new_inode;
 }
@@ -1218,7 +1213,7 @@ KResult Ext2FSInode::remove_child(const StringView& name)
 
     m_lookup_cache.remove(name);
 
-    auto child_inode = fs().get_inode(child_id);
+    auto child_inode = TRY(fs().get_inode(child_id));
     TRY(child_inode->decrement_link_count());
 
     did_remove_child(child_id, name);
@@ -1541,12 +1536,11 @@ KResultOr<NonnullRefPtr<Inode>> Ext2FS::create_inode(Ext2FSInode& parent_inode,
     auto success = write_ext2_inode(inode_id, e2inode);
     VERIFY(success);
 
-    auto new_inode = get_inode({ fsid(), inode_id });
-    VERIFY(new_inode);
+    auto new_inode = TRY(get_inode({ fsid(), inode_id }));
 
     dbgln_if(EXT2_DEBUG, "Ext2FS: Adding inode '{}' (mode {:o}) to parent directory {}", name, mode, parent_inode.index());
     TRY(parent_inode.add_child(*new_inode, name, mode));
-    return new_inode.release_nonnull();
+    return new_inode;
 }
 
 KResult Ext2FSInode::populate_lookup_cache() const
@@ -1586,10 +1580,7 @@ KResultOr<NonnullRefPtr<Inode>> Ext2FSInode::lookup(StringView name)
         inode_index = it->value;
     }
 
-    auto inode = fs().get_inode({ fsid(), inode_index });
-    if (!inode)
-        return ENOENT;
-    return inode.release_nonnull();
+    return fs().get_inode({ fsid(), inode_index });
 }
 
 void Ext2FSInode::one_ref_left()
diff --git a/Kernel/FileSystem/Ext2FileSystem.h b/Kernel/FileSystem/Ext2FileSystem.h
index c73b14230e..0e05f5865f 100644
--- a/Kernel/FileSystem/Ext2FileSystem.h
+++ b/Kernel/FileSystem/Ext2FileSystem.h
@@ -129,7 +129,7 @@ private:
 
     virtual StringView class_name() const override { return "Ext2FS"sv; }
     virtual Ext2FSInode& root_inode() override;
-    RefPtr<Inode> get_inode(InodeIdentifier) const;
+    KResultOr<NonnullRefPtr<Inode>> get_inode(InodeIdentifier) const;
     KResultOr<NonnullRefPtr<Inode>> create_inode(Ext2FSInode& parent_inode, StringView name, mode_t, dev_t, UserID, GroupID);
     KResult create_directory(Ext2FSInode& parent_inode, StringView name, mode_t, UserID, GroupID);
     virtual void flush_writes() override;
diff --git a/Kernel/FileSystem/TmpFS.cpp b/Kernel/FileSystem/TmpFS.cpp
index 7fd8b54276..dbb361a063 100644
--- a/Kernel/FileSystem/TmpFS.cpp
+++ b/Kernel/FileSystem/TmpFS.cpp
@@ -61,14 +61,14 @@ unsigned TmpFS::next_inode_index()
     return m_next_inode_index++;
 }
 
-RefPtr<Inode> TmpFS::get_inode(InodeIdentifier identifier) const
+KResultOr<NonnullRefPtr<Inode>> TmpFS::get_inode(InodeIdentifier identifier) const
 {
     MutexLocker locker(m_lock, Mutex::Mode::Shared);
     VERIFY(identifier.fsid() == fsid());
 
     auto it = m_inodes.find(identifier.index());
     if (it == m_inodes.end())
-        return nullptr;
+        return ENOENT;
     return it->value;
 }
 
@@ -201,13 +201,8 @@ KResultOr<NonnullRefPtr<Inode>> TmpFSInode::lookup(StringView name)
 
     if (name == ".")
         return *this;
-    if (name == "..") {
-        auto inode = fs().get_inode(m_parent);
-        // FIXME: If this cannot fail, we should probably VERIFY here instead.
-        if (!inode)
-            return ENOENT;
-        return inode.release_nonnull();
-    }
+    if (name == "..")
+        return fs().get_inode(m_parent);
 
     auto* child = find_child_by_name(name);
     if (!child)
diff --git a/Kernel/FileSystem/TmpFS.h b/Kernel/FileSystem/TmpFS.h
index 3364a51578..c34290e4b3 100644
--- a/Kernel/FileSystem/TmpFS.h
+++ b/Kernel/FileSystem/TmpFS.h
@@ -34,7 +34,7 @@ private:
     RefPtr<TmpFSInode> m_root_inode;
 
     HashMap<InodeIndex, NonnullRefPtr<TmpFSInode>> m_inodes;
-    RefPtr<Inode> get_inode(InodeIdentifier identifier) const;
+    KResultOr<NonnullRefPtr<Inode>> get_inode(InodeIdentifier identifier) const;
     void register_inode(TmpFSInode&);
     void unregister_inode(InodeIdentifier);