diff --git a/Kernel/Ext2FileSystem.cpp b/Kernel/Ext2FileSystem.cpp index 0c41ce4f21..866974a9b8 100644 --- a/Kernel/Ext2FileSystem.cpp +++ b/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 diff --git a/Kernel/Ext2FileSystem.h b/Kernel/Ext2FileSystem.h index 6f1d8421d1..1b0c8aecd9 100644 --- a/Kernel/Ext2FileSystem.h +++ b/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; diff --git a/Kernel/FileSystem.h b/Kernel/FileSystem.h index 77169cd751..b3081e3269 100644 --- a/Kernel/FileSystem.h +++ b/Kernel/FileSystem.h @@ -15,6 +15,7 @@ #include #include #include +#include static const dword mepoch = 476763780; @@ -96,7 +97,7 @@ public: virtual bool remove_child(const String& name, int& error) = 0; virtual RetainPtr 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(); } diff --git a/Kernel/KResult.h b/Kernel/KResult.h new file mode 100644 index 0000000000..01d3a38bbf --- /dev/null +++ b/Kernel/KResult.h @@ -0,0 +1,51 @@ +#pragma once + +#include +#include + +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 friend class KResultOr; + KResult() { } + + int m_error { 0 }; +}; + +template +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(&m_storage); } + const T& value() const { ASSERT(!m_is_error); return *reinterpret_cast(&m_storage); } + +private: + char m_storage[sizeof(T)] __attribute__((aligned(sizeof(T)))); + KResult m_error; + bool m_is_error { false }; +}; + diff --git a/Kernel/ProcFS.cpp b/Kernel/ProcFS.cpp index 680c46b187..0b26b30102 100644 --- a/Kernel/ProcFS.cpp +++ b/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 false; + return KResult(-EPERM); } ProcFS::ProcFS() diff --git a/Kernel/ProcFS.h b/Kernel/ProcFS.h index ad64feae41..f2b0e0a0e0 100644 --- a/Kernel/ProcFS.h +++ b/Kernel/ProcFS.h @@ -87,7 +87,7 @@ private: virtual bool remove_child(const String& name, int& error) override; virtual RetainPtr 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(Inode::fs()); } const ProcFS& fs() const { return static_cast(Inode::fs()); } diff --git a/Kernel/Process.cpp b/Kernel/Process.cpp index bc0c69d977..11d1acd279 100644 --- a/Kernel/Process.cpp +++ b/Kernel/Process.cpp @@ -1136,10 +1136,7 @@ int Process::sys$utime(const char* pathname, const utimbuf* buf) mtime = now; atime = now; } - int error; - if (!VFS::the().utime(String(pathname), error, cwd_inode(), atime, mtime)) - return error; - return 0; + return VFS::the().utime(String(pathname), cwd_inode(), atime, mtime); } 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; - if (!VFS::the().mkdir(String(pathname, pathname_length), mode & ~umask(), cwd_inode(), error)) - return error; - return 0; + return VFS::the().mkdir(String(pathname, pathname_length), mode & ~umask(), cwd_inode()); } 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; - if (!VFS::the().chmod(String(pathname), mode, cwd_inode(), error)) - return error; - return 0; + return VFS::the().chmod(String(pathname), mode, cwd_inode()); } void Process::finalize() diff --git a/Kernel/SyntheticFileSystem.cpp b/Kernel/SyntheticFileSystem.cpp index 1ed6e6eb50..bfdf6a8c8e 100644 --- a/Kernel/SyntheticFileSystem.cpp +++ b/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 false; + return KResult(-EPERM); } diff --git a/Kernel/SyntheticFileSystem.h b/Kernel/SyntheticFileSystem.h index cfe04180d7..fe4b312941 100644 --- a/Kernel/SyntheticFileSystem.h +++ b/Kernel/SyntheticFileSystem.h @@ -67,7 +67,7 @@ private: virtual bool remove_child(const String& name, int& error) override; virtual RetainPtr 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; diff --git a/Kernel/VirtualFileSystem.cpp b/Kernel/VirtualFileSystem.cpp index 2f24779ccc..a144640bb6 100644 --- a/Kernel/VirtualFileSystem.cpp +++ b/Kernel/VirtualFileSystem.cpp @@ -42,7 +42,7 @@ bool VFS::mount(RetainPtr&& 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 VFS::open(RetainPtr&& 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()) { - error = -EROFS; - return false; - } - if (inode.metadata().uid != current->euid()) { - error = -EACCES; - return false; - } + if (inode.fs().is_readonly()) + return KResult(-EROFS); + if (inode.metadata().uid != current->euid()) + return KResult(-EACCES); error = inode.set_atime(atime); if (error) - return false; + return KResult(error); error = inode.set_mtime(mtime); if (error) - return false; - return true; + return KResult(error); + 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 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 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 parent_inode; - auto existing_dir = resolve_path_to_inode(path, base, error, &parent_inode); - if (existing_dir) { - error = -EEXIST; - return false; - } - if (!parent_inode) { - error = -ENOENT; - return false; - } - if (error != -ENOENT) { - return false; - } - if (!parent_inode->metadata().may_write(*current)) { - error = -EACCES; - return false; - } + auto result = resolve_path_to_inode(path, base, &parent_inode); + if (!result.is_error()) + return KResult(-EEXIST); + if (!parent_inode) + return KResult(-ENOENT); + if (result.error() != -ENOENT) + return result.error(); + + if (!parent_inode->metadata().may_write(*current)) + return KResult(-EACCES); 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) { - error = 0; - return true; - } - return false; + if (new_dir) + return KSuccess; + return KResult(error); } -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 = resolve_path_to_inode(path, base, error); - if (!inode) - return false; + auto inode_or_error = resolve_path_to_inode(path, base); + if (inode_or_error.is_error()) + return inode_or_error.error(); + auto inode = inode_or_error.value(); - if (inode->fs().is_readonly()) { - error = -EROFS; - return false; - } + if (inode->fs().is_readonly()) + return KResult(-EROFS); - if (current->euid() != inode->metadata().uid) { - error = -EPERM; - return false; - } + if (current->euid() != inode->metadata().uid) + return KResult(-EPERM); // 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 false; - error = 0; - return true; + return inode->chmod(mode); +} + +KResultOr> VFS::resolve_path_to_inode(const String& path, Inode& base, RetainPtr* 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 VFS::resolve_path_to_inode(const String& path, Inode& base, int& error, RetainPtr* parent_inode) @@ -316,7 +312,7 @@ RetainPtr 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 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 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 VFS::resolve_path(const String& path, InodeIdentifier base, int options, InodeIdentifier* parent_id) { - if (path.is_empty()) { - error = -EINVAL; - return { }; - } + if (path.is_empty()) + return KResult(-EINVAL); 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 { }; + return KResult(-EIO); } 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 { }; - } - if (!metadata.may_execute(*current)) { - error = -EACCES; - return { }; + return KResult(-ENOTDIR); } + 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 { }; + return KResult(-ENOENT); } #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) { - error = -ELOOP; - return { }; - } + if (options & O_NOFOLLOW) + return KResult(-ELOOP); if (options & O_NOFOLLOW_NOERROR) return crumb_id; } - crumb_id = resolve_symbolic_link(parent, *crumb_inode, error); - if (!crumb_id.is_valid()) { - kprintf("Symbolic link resolution failed :(\n"); - return { }; - } + auto result = resolve_symbolic_link(parent, *crumb_inode); + if (result.is_error()) + return KResult(-ENOENT); + 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&& guest_fs) : m_host(host) , m_guest(guest_fs->root_inode()) diff --git a/Kernel/VirtualFileSystem.h b/Kernel/VirtualFileSystem.h index b7e9529443..dc41b2dacf 100644 --- a/Kernel/VirtualFileSystem.h +++ b/Kernel/VirtualFileSystem.h @@ -10,6 +10,7 @@ #include "InodeMetadata.h" #include "Limits.h" #include "FileSystem.h" +#include #define O_RDONLY 0 #define O_WRONLY 1 @@ -64,13 +65,13 @@ public: RetainPtr open(RetainPtr&&, int& error, int options); RetainPtr open(const String& path, int& error, int options, mode_t mode, Inode& base); RetainPtr 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); - 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 resolve_path(const String& path, InodeIdentifier base, int options = 0, InodeIdentifier* parent_id = nullptr); RetainPtr resolve_path_to_inode(const String& path, Inode& base, int& error, RetainPtr* parent_id = nullptr); - InodeIdentifier resolve_symbolic_link(InodeIdentifier base, Inode& symlink_inode, int& error); + KResultOr> resolve_path_to_inode(const String& path, Inode& base, RetainPtr* parent_id = nullptr); + KResultOr resolve_symbolic_link(InodeIdentifier base, Inode& symlink_inode); Mount* find_mount_for_host(InodeIdentifier); Mount* find_mount_for_guest(InodeIdentifier); diff --git a/LibC/errno_numbers.h b/LibC/errno_numbers.h index b441460a57..03cd9ce433 100644 --- a/LibC/errno_numbers.h +++ b/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