Kernel: Add KResult and KResultOr<T> classes.

The idea here is to combine a potential syscall error code with an arbitrary
type in the case of success. I feel like this will end up much less error
prone than returning some arbitrary type that kinda sorta has bool semantics
(but sometimes not really) and passing the error through an out-param.

This patch only converts a few syscalls to using it. More to come.

Kernel/Ext2FileSystem.cpp

@@ -1350,15 +1350,14 @@ size_t Ext2FSInode::directory_entry_count() const
     return m_lookup_cache.size();
 }
 
-bool Ext2FSInode::chmod(mode_t mode, int& error)
+KResult Ext2FSInode::chmod(mode_t mode)
 {
     LOCKER(m_lock);
-    error = 0;
     if (m_raw_inode.i_mode == mode)
-        return true;
+        return KSuccess;
     m_raw_inode.i_mode = mode;
     set_metadata_dirty(true);
-    return true;
+    return KSuccess;
 }
 
 unsigned Ext2FS::total_block_count() const

Kernel/Ext2FileSystem.h

@@ -41,7 +41,7 @@ private:
     virtual int increment_link_count() override;
     virtual int decrement_link_count() override;
     virtual size_t directory_entry_count() const override;
-    virtual bool chmod(mode_t, int& error) override;
+    virtual KResult chmod(mode_t) override;
 
     void populate_lookup_cache() const;
 

Kernel/FileSystem.h

@@ -15,6 +15,7 @@
 #include <AK/kstdio.h>
 #include <AK/Lock.h>
 #include <AK/WeakPtr.h>
+#include <Kernel/KResult.h>
 
 static const dword mepoch = 476763780;
 
@@ -96,7 +97,7 @@ public:
     virtual bool remove_child(const String& name, int& error) = 0;
     virtual RetainPtr<Inode> parent() const = 0;
     virtual size_t directory_entry_count() const = 0;
-    virtual bool chmod(mode_t, int& error) = 0;
+    virtual KResult chmod(mode_t) = 0;
 
     LocalSocket* socket() { return m_socket.ptr(); }
     const LocalSocket* socket() const { return m_socket.ptr(); }

Kernel/KResult.h

@@ -0,0 +1,51 @@
+#pragma once
+
+#include <AK/Assertions.h>
+#include <LibC/errno_numbers.h>
+
+enum KSuccessTag { KSuccess };
+
+class KResult {
+public:
+    explicit KResult(__errno_value e) : m_error(-e) { }
+    explicit KResult(int negative_e) : m_error(negative_e) { ASSERT(negative_e <= 0); }
+    KResult(KSuccessTag) : m_error(0) { }
+    operator int() const { return m_error; }
+
+private:
+    template<typename T> friend class KResultOr;
+    KResult() { }
+
+    int m_error { 0 };
+};
+
+template<typename T>
+class alignas(T) KResultOr {
+public:
+    KResultOr(KResult error)
+        : m_error(error)
+        , m_is_error(true)
+    { }
+
+    KResultOr(T&& value)
+    {
+        new (&m_storage) T(move(value));
+    }
+
+    ~KResultOr()
+    {
+        if (!m_is_error)
+            value().~T();
+    }
+
+    bool is_error() const { return m_is_error; }
+    KResult error() const { ASSERT(m_is_error); return m_error; }
+    T& value() { ASSERT(!m_is_error); return *reinterpret_cast<T*>(&m_storage); }
+    const T& value() const { ASSERT(!m_is_error); return *reinterpret_cast<T*>(&m_storage); }
+
+private:
+    char m_storage[sizeof(T)] __attribute__((aligned(sizeof(T))));
+    KResult m_error;
+    bool m_is_error { false };
+};
+

Kernel/ProcFS.cpp

@@ -1049,12 +1049,16 @@ ssize_t ProcFSInode::write_bytes(off_t offset, size_t size, const byte* buffer,
 
 bool ProcFSInode::add_child(InodeIdentifier child_id, const String& name, byte file_type, int& error)
 {
+    (void)child_id;
+    (void)name;
+    (void)file_type;
     error = -EPERM;
     return false;
 }
 
 bool ProcFSInode::remove_child(const String& name, int& error)
 {
+    (void)name;
     error = -EPERM;
     return false;
 }
@@ -1074,10 +1078,9 @@ size_t ProcFSInode::directory_entry_count() const
     return count;
 }
 
-bool ProcFSInode::chmod(mode_t, int& error)
+KResult ProcFSInode::chmod(mode_t)
 {
-    error = -EPERM;
+    return KResult(-EPERM);
-    return false;
 }
 
 ProcFS::ProcFS()

Kernel/ProcFS.h

@@ -87,7 +87,7 @@ private:
     virtual bool remove_child(const String& name, int& error) override;
     virtual RetainPtr<Inode> parent() const override;
     virtual size_t directory_entry_count() const override;
-    virtual bool chmod(mode_t, int& error) override;
+    virtual KResult chmod(mode_t) override;
 
     ProcFS& fs() { return static_cast<ProcFS&>(Inode::fs()); }
     const ProcFS& fs() const { return static_cast<const ProcFS&>(Inode::fs()); }

Kernel/Process.cpp

@@ -1136,10 +1136,7 @@ int Process::sys$utime(const char* pathname, const utimbuf* buf)
         mtime = now;
         atime = now;
     }
-    int error;
+    return VFS::the().utime(String(pathname), cwd_inode(), atime, mtime);
-    if (!VFS::the().utime(String(pathname), error, cwd_inode(), atime, mtime))
-        return error;
-    return 0;
 }
 
 int Process::sys$access(const char* pathname, int mode)
@@ -1931,10 +1928,7 @@ int Process::sys$mkdir(const char* pathname, mode_t mode)
         return -EINVAL;
     if (pathname_length >= 255)
         return -ENAMETOOLONG;
-    int error;
+    return VFS::the().mkdir(String(pathname, pathname_length), mode & ~umask(), cwd_inode());
-    if (!VFS::the().mkdir(String(pathname, pathname_length), mode & ~umask(), cwd_inode(), error))
-        return error;
-    return 0;
 }
 
 clock_t Process::sys$times(tms* times)
@@ -2143,10 +2137,7 @@ int Process::sys$chmod(const char* pathname, mode_t mode)
 {
     if (!validate_read_str(pathname))
         return -EFAULT;
-    int error;
+    return VFS::the().chmod(String(pathname), mode, cwd_inode());
-    if (!VFS::the().chmod(String(pathname), mode, cwd_inode(), error))
-        return error;
-    return 0;
 }
 
 void Process::finalize()

Kernel/SyntheticFileSystem.cpp

@@ -311,8 +311,7 @@ size_t SynthFSInode::directory_entry_count() const
     return m_children.size() + 2;
 }
 
-bool SynthFSInode::chmod(mode_t, int& error)
+KResult SynthFSInode::chmod(mode_t)
 {
-    error = -EPERM;
+    return KResult(-EPERM);
-    return false;
 }

Kernel/SyntheticFileSystem.h

@@ -67,7 +67,7 @@ private:
     virtual bool remove_child(const String& name, int& error) override;
     virtual RetainPtr<Inode> parent() const override;
     virtual size_t directory_entry_count() const override;
-    virtual bool chmod(mode_t, int& error) override;
+    virtual KResult chmod(mode_t) override;
 
     SynthFS& fs();
     const SynthFS& fs() const;

Kernel/VirtualFileSystem.cpp

@@ -42,7 +42,7 @@ bool VFS::mount(RetainPtr<FS>&& file_system, const String& path)
 {
     ASSERT(file_system);
     int error;
-    auto inode = resolve_path(path, root_inode_id(), error);
+    auto inode = old_resolve_path(path, root_inode_id(), error);
     if (!inode.is_valid()) {
         kprintf("VFS: mount can't resolve mount point '%s'\n", path.characters());
         return false;
@@ -131,32 +131,29 @@ RetainPtr<FileDescriptor> VFS::open(RetainPtr<Device>&& device, int& error, int
     return FileDescriptor::create(move(device));
 }
 
-bool VFS::utime(const String& path, int& error, Inode& base, time_t atime, time_t mtime)
+KResult VFS::utime(const String& path, Inode& base, time_t atime, time_t mtime)
 {
+    int error;
     auto descriptor = VFS::the().open(move(path), error, 0, 0, base);
     if (!descriptor)
-        return false;
+        return KResult(error);
     auto& inode = *descriptor->inode();
-    if (inode.fs().is_readonly()) {
+    if (inode.fs().is_readonly())
-        error = -EROFS;
+        return KResult(-EROFS);
-        return false;
+    if (inode.metadata().uid != current->euid())
-    }
+        return KResult(-EACCES);
-    if (inode.metadata().uid != current->euid()) {
-        error = -EACCES;
-        return false;
-    }
     error = inode.set_atime(atime);
     if (error)
-        return false;
+        return KResult(error);
     error = inode.set_mtime(mtime);
     if (error)
-        return false;
+        return KResult(error);
-    return true;
+    return KSuccess;
 }
 
 bool VFS::stat(const String& path, int& error, int options, Inode& base, struct stat& statbuf)
 {
-    auto inode_id = resolve_path(path, base.identifier(), error, options);
+    auto inode_id = old_resolve_path(path, base.identifier(), error, options);
     if (!inode_id.is_valid())
         return false;
     error = FileDescriptor::create(get_inode(inode_id))->fstat(&statbuf);
@@ -167,7 +164,7 @@ bool VFS::stat(const String& path, int& error, int options, Inode& base, struct
 
 RetainPtr<FileDescriptor> VFS::open(const String& path, int& error, int options, mode_t mode, Inode& base)
 {
-    auto inode_id = resolve_path(path, base.identifier(), error, options);
+    auto inode_id = old_resolve_path(path, base.identifier(), error, options);
     auto inode = get_inode(inode_id);
     if (options & O_CREAT) {
         if (!inode)
@@ -248,63 +245,62 @@ RetainPtr<FileDescriptor> VFS::create(const String& path, int& error, int option
     return FileDescriptor::create(move(new_file));
 }
 
-bool VFS::mkdir(const String& path, mode_t mode, Inode& base, int& error)
+KResult VFS::mkdir(const String& path, mode_t mode, Inode& base)
 {
-    error = -EWHYTHO;
-
     RetainPtr<Inode> parent_inode;
-    auto existing_dir = resolve_path_to_inode(path, base, error, &parent_inode);
+    auto result = resolve_path_to_inode(path, base, &parent_inode);
-    if (existing_dir) {
+    if (!result.is_error())
-        error = -EEXIST;
+        return KResult(-EEXIST);
-        return false;
+    if (!parent_inode)
-    }
+        return KResult(-ENOENT);
-    if (!parent_inode) {
+    if (result.error() != -ENOENT)
-        error = -ENOENT;
+        return result.error();
-        return false;
+
-    }
+    if (!parent_inode->metadata().may_write(*current))
-    if (error != -ENOENT) {
+        return KResult(-EACCES);
-        return false;
-    }
-    if (!parent_inode->metadata().may_write(*current)) {
-        error = -EACCES;
-        return false;
-    }
 
     FileSystemPath p(path);
     dbgprintf("VFS::mkdir: '%s' in %u:%u\n", p.basename().characters(), parent_inode->fsid(), parent_inode->index());
+    int error;
     auto new_dir = parent_inode->fs().create_directory(parent_inode->identifier(), p.basename(), mode, error);
-    if (new_dir) {
+    if (new_dir)
-        error = 0;
+        return KSuccess;
-        return true;
+    return KResult(error);
-    }
-    return false;
 }
 
-bool VFS::chmod(const String& path, mode_t mode, Inode& base, int& error)
+KResult VFS::chmod(const String& path, mode_t mode, Inode& base)
 {
-    error = -EWHYTHO;
+    auto inode_or_error = resolve_path_to_inode(path, base);
-    auto inode = resolve_path_to_inode(path, base, error);
+    if (inode_or_error.is_error())
-    if (!inode)
+        return inode_or_error.error();
-        return false;
+    auto inode = inode_or_error.value();
 
-    if (inode->fs().is_readonly()) {
+    if (inode->fs().is_readonly())
-        error = -EROFS;
+        return KResult(-EROFS);
-        return false;
-    }
 
-    if (current->euid() != inode->metadata().uid) {
+    if (current->euid() != inode->metadata().uid)
-        error = -EPERM;
+        return KResult(-EPERM);
-        return false;
-    }
 
     // Only change the permission bits.
     mode = (inode->mode() & ~04777) | (mode & 04777);
 
     kprintf("VFS::chmod(): %u:%u mode %o\n", inode->fsid(), inode->index(), mode);
-    if (!inode->chmod(mode, error))
+    return inode->chmod(mode);
-        return false;
+}
-    error = 0;
+
-    return true;
+KResultOr<RetainPtr<Inode>> VFS::resolve_path_to_inode(const String& path, Inode& base, RetainPtr<Inode>* parent_inode)
+{
+    // FIXME: This won't work nicely across mount boundaries.
+    FileSystemPath p(path);
+    if (!p.is_valid())
+        return KResult(-EINVAL);
+    InodeIdentifier parent_id;
+    auto result = resolve_path(path, base.identifier(), 0, &parent_id);
+    if (parent_inode && parent_id.is_valid())
+        *parent_inode = get_inode(parent_id);
+    if (result.is_error())
+        return result.error();
+    return get_inode(result.value());
 }
 
 RetainPtr<Inode> VFS::resolve_path_to_inode(const String& path, Inode& base, int& error, RetainPtr<Inode>* parent_inode)
@@ -316,7 +312,7 @@ RetainPtr<Inode> VFS::resolve_path_to_inode(const String& path, Inode& base, int
         return nullptr;
     }
     InodeIdentifier parent_id;
-    auto inode_id = resolve_path(path, base.identifier(), error, 0, &parent_id);
+    auto inode_id = old_resolve_path(path, base.identifier(), error, 0, &parent_id);
     if (parent_inode && parent_id.is_valid())
         *parent_inode = get_inode(parent_id);
     if (!inode_id.is_valid()) {
@@ -437,16 +433,16 @@ bool VFS::rmdir(const String& path, Inode& base, int& error)
     return true;
 }
 
-InodeIdentifier VFS::resolve_symbolic_link(InodeIdentifier base, Inode& symlink_inode, int& error)
+KResultOr<InodeIdentifier> VFS::resolve_symbolic_link(InodeIdentifier base, Inode& symlink_inode)
 {
     auto symlink_contents = symlink_inode.read_entire();
     if (!symlink_contents)
-        return { };
+        return KResult(-ENOENT);
     auto linkee = String((const char*)symlink_contents.pointer(), symlink_contents.size());
 #ifdef VFS_DEBUG
     kprintf("linkee (%s)(%u) from %u:%u\n", linkee.characters(), linkee.length(), base.fsid(), base.index());
 #endif
-    return resolve_path(linkee, base, error);
+    return resolve_path(linkee, base);
 }
 
 RetainPtr<Inode> VFS::get_inode(InodeIdentifier inode_id)
@@ -477,7 +473,7 @@ String VFS::absolute_path(Inode& core_inode)
 
         InodeIdentifier parent_id;
         if (inode->is_directory()) {
-            parent_id = resolve_path("..", inode->identifier(), error);
+            parent_id = old_resolve_path("..", inode->identifier(), error);
         } else {
             parent_id = inode->parent()->identifier();
         }
@@ -500,12 +496,10 @@ String VFS::absolute_path(Inode& core_inode)
     return builder.to_string();
 }
 
-InodeIdentifier VFS::resolve_path(const String& path, InodeIdentifier base, int& error, int options, InodeIdentifier* parent_id)
+KResultOr<InodeIdentifier> VFS::resolve_path(const String& path, InodeIdentifier base, int options, InodeIdentifier* parent_id)
 {
-    if (path.is_empty()) {
+    if (path.is_empty())
-        error = -EINVAL;
+        return KResult(-EINVAL);
-        return { };
-    }
 
     auto parts = path.split('/');
     InodeIdentifier crumb_id;
@@ -528,29 +522,24 @@ InodeIdentifier VFS::resolve_path(const String& path, InodeIdentifier base, int&
 #ifdef VFS_DEBUG
             kprintf("invalid metadata\n");
 #endif
-            error = -EIO;
+            return KResult(-EIO);
-            return { };
         }
         auto metadata = crumb_inode->metadata();
         if (!metadata.is_directory()) {
 #ifdef VFS_DEBUG
             kprintf("parent of <%s> not directory, it's inode %u:%u / %u:%u, mode: %u, size: %u\n", part.characters(), crumb_id.fsid(), crumb_id.index(), metadata.inode.fsid(), metadata.inode.index(), metadata.mode, metadata.size);
 #endif
-            error = -ENOTDIR;
+            return KResult(-ENOTDIR);
-            return { };
-        }
-        if (!metadata.may_execute(*current)) {
-            error = -EACCES;
-            return { };
         }
+        if (!metadata.may_execute(*current))
+            return KResult(-EACCES);
         auto parent = crumb_id;
         crumb_id = crumb_inode->lookup(part);
         if (!crumb_id.is_valid()) {
 #ifdef VFS_DEBUG
             kprintf("child <%s>(%u) not found in directory, %02u:%08u\n", part.characters(), part.length(), parent.fsid(), parent.index());
 #endif
-            error = -ENOENT;
+            return KResult(-ENOENT);
-            return { };
         }
 #ifdef VFS_DEBUG
         kprintf("<%s> %u:%u\n", part.characters(), crumb_id.fsid(), crumb_id.index());
@@ -581,24 +570,32 @@ InodeIdentifier VFS::resolve_path(const String& path, InodeIdentifier base, int&
         }
         if (metadata.is_symlink()) {
             if (i == parts.size() - 1) {
-                if (options & O_NOFOLLOW) {
+                if (options & O_NOFOLLOW)
-                    error = -ELOOP;
+                    return KResult(-ELOOP);
-                    return { };
-                }
                 if (options & O_NOFOLLOW_NOERROR)
                     return crumb_id;
             }
-            crumb_id = resolve_symbolic_link(parent, *crumb_inode, error);
+            auto result = resolve_symbolic_link(parent, *crumb_inode);
-            if (!crumb_id.is_valid()) {
+            if (result.is_error())
-                kprintf("Symbolic link resolution failed :(\n");
+                return KResult(-ENOENT);
-                return { };
+            crumb_id = result.value();
-            }
+            ASSERT(crumb_id.is_valid());
         }
     }
 
     return crumb_id;
 }
 
+InodeIdentifier VFS::old_resolve_path(const String& path, InodeIdentifier base, int& error, int options, InodeIdentifier* parent_id)
+{
+    auto result = resolve_path(path, base, options, parent_id);
+    if (result.is_error()) {
+        error = result.error();
+        return { };
+    }
+    return result.value();
+}
+
 VFS::Mount::Mount(InodeIdentifier host, RetainPtr<FS>&& guest_fs)
     : m_host(host)
     , m_guest(guest_fs->root_inode())

Kernel/VirtualFileSystem.h

@@ -10,6 +10,7 @@
 #include "InodeMetadata.h"
 #include "Limits.h"
 #include "FileSystem.h"
+#include <Kernel/KResult.h>
 
 #define O_RDONLY 0
 #define O_WRONLY 1
@@ -64,13 +65,13 @@ public:
     RetainPtr<FileDescriptor> open(RetainPtr<Device>&&, int& error, int options);
     RetainPtr<FileDescriptor> open(const String& path, int& error, int options, mode_t mode, Inode& base);
     RetainPtr<FileDescriptor> create(const String& path, int& error, int options, mode_t mode, Inode& base);
-    bool mkdir(const String& path, mode_t mode, Inode& base, int& error);
+    KResult mkdir(const String& path, mode_t mode, Inode& base);
     bool link(const String& old_path, const String& new_path, Inode& base, int& error);
     bool unlink(const String& path, Inode& base, int& error);
     bool rmdir(const String& path, Inode& base, int& error);
-    bool chmod(const String& path, mode_t, Inode& base, int& error);
+    KResult chmod(const String& path, mode_t, Inode& base);
     bool stat(const String& path, int& error, int options, Inode& base, struct stat&);
-    bool utime(const String& path, int& error, Inode& base, time_t atime, time_t mtime);
+    KResult utime(const String& path, Inode& base, time_t atime, time_t mtime);
 
     void register_device(Device&);
     void unregister_device(Device&);
@@ -97,9 +98,11 @@ private:
     bool is_vfs_root(InodeIdentifier) const;
 
     void traverse_directory_inode(Inode&, Function<bool(const FS::DirectoryEntry&)>);
-    InodeIdentifier resolve_path(const String& path, InodeIdentifier base, int& error, int options = 0, InodeIdentifier* parent_id = nullptr);
+    InodeIdentifier old_resolve_path(const String& path, InodeIdentifier base, int& error, int options = 0, InodeIdentifier* parent_id = nullptr);
+    KResultOr<InodeIdentifier> resolve_path(const String& path, InodeIdentifier base, int options = 0, InodeIdentifier* parent_id = nullptr);
     RetainPtr<Inode> resolve_path_to_inode(const String& path, Inode& base, int& error, RetainPtr<Inode>* parent_id = nullptr);
-    InodeIdentifier resolve_symbolic_link(InodeIdentifier base, Inode& symlink_inode, int& error);
+    KResultOr<RetainPtr<Inode>> resolve_path_to_inode(const String& path, Inode& base, RetainPtr<Inode>* parent_id = nullptr);
+    KResultOr<InodeIdentifier> resolve_symbolic_link(InodeIdentifier base, Inode& symlink_inode);
 
     Mount* find_mount_for_host(InodeIdentifier);
     Mount* find_mount_for_guest(InodeIdentifier);

LibC/errno_numbers.h

@@ -74,7 +74,7 @@
     __ERROR(EMAXERRNO,      "The highest errno +1 :^)")
 
 
-enum __errno_values {
+enum __errno_value {
 #undef __ENUMERATE_ERROR
 #define __ERROR(a, b) a,
     __ENUMERATE_ALL_ERRORS