From 234c6ae32d5432bdf836e510fc67756ff750c295 Mon Sep 17 00:00:00 2001 From: Brian Gianforcaro Date: Sat, 1 May 2021 14:29:39 -0700 Subject: [PATCH] Kernel: Change Inode::{read/write}_bytes interface to KResultOr The error handling in all these cases was still using the old style negative values to indicate errors. We have a nicer solution for this now with KResultOr. This change switches the interface and then all implementers to use the new style. --- Kernel/FileSystem/DevFS.cpp | 19 +++++++------- Kernel/FileSystem/DevFS.h | 12 ++++----- Kernel/FileSystem/DevPtsFS.cpp | 4 +-- Kernel/FileSystem/DevPtsFS.h | 4 +-- Kernel/FileSystem/Ext2FileSystem.cpp | 34 ++++++++++++------------- Kernel/FileSystem/Ext2FileSystem.h | 4 +-- Kernel/FileSystem/Inode.cpp | 7 ++--- Kernel/FileSystem/Inode.h | 4 +-- Kernel/FileSystem/InodeFile.cpp | 15 ++++++----- Kernel/FileSystem/Plan9FileSystem.cpp | 10 ++++---- Kernel/FileSystem/Plan9FileSystem.h | 4 +-- Kernel/FileSystem/ProcFS.cpp | 14 +++++----- Kernel/FileSystem/ProcFS.h | 8 +++--- Kernel/FileSystem/TmpFS.cpp | 10 ++++---- Kernel/FileSystem/TmpFS.h | 4 +-- Kernel/FileSystem/VirtualFileSystem.cpp | 6 ++--- Kernel/Syscalls/mmap.cpp | 4 +-- Kernel/VM/Region.cpp | 7 ++--- 18 files changed, 88 insertions(+), 82 deletions(-) diff --git a/Kernel/FileSystem/DevFS.cpp b/Kernel/FileSystem/DevFS.cpp index 71025d1d8c..8f0507c805 100644 --- a/Kernel/FileSystem/DevFS.cpp +++ b/Kernel/FileSystem/DevFS.cpp @@ -80,7 +80,8 @@ DevFSInode::DevFSInode(DevFS& fs) : Inode(fs, fs.allocate_inode_index()) { } -ssize_t DevFSInode::read_bytes(off_t, ssize_t, UserOrKernelBuffer&, FileDescription*) const + +KResultOr DevFSInode::read_bytes(off_t, ssize_t, UserOrKernelBuffer&, FileDescription*) const { VERIFY_NOT_REACHED(); } @@ -99,7 +100,7 @@ void DevFSInode::flush_metadata() { } -ssize_t DevFSInode::write_bytes(off_t, ssize_t, const UserOrKernelBuffer&, FileDescription*) +KResultOr DevFSInode::write_bytes(off_t, ssize_t, const UserOrKernelBuffer&, FileDescription*) { VERIFY_NOT_REACHED(); } @@ -151,13 +152,13 @@ DevFSLinkInode::DevFSLinkInode(DevFS& fs, String name) , m_name(name) { } -ssize_t DevFSLinkInode::read_bytes(off_t offset, ssize_t, UserOrKernelBuffer& buffer, FileDescription*) const +KResultOr DevFSLinkInode::read_bytes(off_t offset, ssize_t, UserOrKernelBuffer& buffer, FileDescription*) const { Locker locker(m_lock); VERIFY(offset == 0); VERIFY(!m_link.is_null()); if (!buffer.write(((const u8*)m_link.substring_view(0).characters_without_null_termination()) + offset, m_link.length())) - return -EFAULT; + return EFAULT; return m_link.length(); } InodeMetadata DevFSLinkInode::metadata() const @@ -172,7 +173,7 @@ InodeMetadata DevFSLinkInode::metadata() const metadata.mtime = mepoch; return metadata; } -ssize_t DevFSLinkInode::write_bytes(off_t offset, ssize_t count, const UserOrKernelBuffer& buffer, FileDescription*) +KResultOr DevFSLinkInode::write_bytes(off_t offset, ssize_t count, const UserOrKernelBuffer& buffer, FileDescription*) { Locker locker(m_lock); VERIFY(offset == 0); @@ -345,7 +346,7 @@ String DevFSDeviceInode::name() const return m_cached_name; } -ssize_t DevFSDeviceInode::read_bytes(off_t offset, ssize_t count, UserOrKernelBuffer& buffer, FileDescription* description) const +KResultOr DevFSDeviceInode::read_bytes(off_t offset, ssize_t count, UserOrKernelBuffer& buffer, FileDescription* description) const { Locker locker(m_lock); VERIFY(!!description); @@ -353,7 +354,7 @@ ssize_t DevFSDeviceInode::read_bytes(off_t offset, ssize_t count, UserOrKernelBu return 0; auto nread = const_cast(*m_attached_device).read(*description, offset, buffer, count); if (nread.is_error()) - return -EIO; + return EIO; return nread.value(); } @@ -371,7 +372,7 @@ InodeMetadata DevFSDeviceInode::metadata() const metadata.minor_device = m_attached_device->minor(); return metadata; } -ssize_t DevFSDeviceInode::write_bytes(off_t offset, ssize_t count, const UserOrKernelBuffer& buffer, FileDescription* description) +KResultOr DevFSDeviceInode::write_bytes(off_t offset, ssize_t count, const UserOrKernelBuffer& buffer, FileDescription* description) { Locker locker(m_lock); VERIFY(!!description); @@ -379,7 +380,7 @@ ssize_t DevFSDeviceInode::write_bytes(off_t offset, ssize_t count, const UserOrK return 0; auto nread = const_cast(*m_attached_device).write(*description, offset, buffer, count); if (nread.is_error()) - return -EIO; + return EIO; return nread.value(); } diff --git a/Kernel/FileSystem/DevFS.h b/Kernel/FileSystem/DevFS.h index ea06194154..02cf5abe37 100644 --- a/Kernel/FileSystem/DevFS.h +++ b/Kernel/FileSystem/DevFS.h @@ -57,11 +57,11 @@ public: protected: DevFSInode(DevFS&); - virtual ssize_t read_bytes(off_t, ssize_t, UserOrKernelBuffer& buffer, FileDescription*) const override; + virtual KResultOr read_bytes(off_t, ssize_t, UserOrKernelBuffer& buffer, FileDescription*) 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 UserOrKernelBuffer& buffer, FileDescription*) override; + virtual KResultOr write_bytes(off_t, ssize_t, const UserOrKernelBuffer& buffer, FileDescription*) 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; virtual KResult remove_child(const StringView& name) override; @@ -83,9 +83,9 @@ private: String determine_name() const; DevFSDeviceInode(DevFS&, const Device&); // ^Inode - virtual ssize_t read_bytes(off_t, ssize_t, UserOrKernelBuffer& buffer, FileDescription*) const override; + virtual KResultOr read_bytes(off_t, ssize_t, UserOrKernelBuffer& buffer, FileDescription*) const override; virtual InodeMetadata metadata() const override; - virtual ssize_t write_bytes(off_t, ssize_t, const UserOrKernelBuffer& buffer, FileDescription*) override; + virtual KResultOr write_bytes(off_t, ssize_t, const UserOrKernelBuffer& buffer, FileDescription*) override; virtual KResult chown(uid_t, gid_t) override; NonnullRefPtr m_attached_device; @@ -106,9 +106,9 @@ public: protected: DevFSLinkInode(DevFS&, String); // ^Inode - virtual ssize_t read_bytes(off_t, ssize_t, UserOrKernelBuffer& buffer, FileDescription*) const override; + virtual KResultOr read_bytes(off_t, ssize_t, UserOrKernelBuffer& buffer, FileDescription*) const override; virtual InodeMetadata metadata() const override; - virtual ssize_t write_bytes(off_t, ssize_t, const UserOrKernelBuffer& buffer, FileDescription*) override; + virtual KResultOr write_bytes(off_t, ssize_t, const UserOrKernelBuffer& buffer, FileDescription*) override; const String m_name; String m_link; diff --git a/Kernel/FileSystem/DevPtsFS.cpp b/Kernel/FileSystem/DevPtsFS.cpp index b1df786fe5..7d5204b880 100644 --- a/Kernel/FileSystem/DevPtsFS.cpp +++ b/Kernel/FileSystem/DevPtsFS.cpp @@ -100,12 +100,12 @@ DevPtsFSInode::~DevPtsFSInode() { } -ssize_t DevPtsFSInode::read_bytes(off_t, ssize_t, UserOrKernelBuffer&, FileDescription*) const +KResultOr DevPtsFSInode::read_bytes(off_t, ssize_t, UserOrKernelBuffer&, FileDescription*) const { VERIFY_NOT_REACHED(); } -ssize_t DevPtsFSInode::write_bytes(off_t, ssize_t, const UserOrKernelBuffer&, FileDescription*) +KResultOr DevPtsFSInode::write_bytes(off_t, ssize_t, const UserOrKernelBuffer&, FileDescription*) { VERIFY_NOT_REACHED(); } diff --git a/Kernel/FileSystem/DevPtsFS.h b/Kernel/FileSystem/DevPtsFS.h index 1dc4ce8ce8..891bbf2ea2 100644 --- a/Kernel/FileSystem/DevPtsFS.h +++ b/Kernel/FileSystem/DevPtsFS.h @@ -47,12 +47,12 @@ private: DevPtsFSInode(DevPtsFS&, InodeIndex, SlavePTY*); // ^Inode - virtual ssize_t read_bytes(off_t, ssize_t, UserOrKernelBuffer& buffer, FileDescription*) const override; + virtual KResultOr read_bytes(off_t, ssize_t, UserOrKernelBuffer& buffer, FileDescription*) const override; virtual InodeMetadata metadata() 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 UserOrKernelBuffer& buffer, FileDescription*) override; + virtual KResultOr write_bytes(off_t, ssize_t, const UserOrKernelBuffer& buffer, FileDescription*) 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; virtual KResult remove_child(const StringView& name) override; diff --git a/Kernel/FileSystem/Ext2FileSystem.cpp b/Kernel/FileSystem/Ext2FileSystem.cpp index d9b1a69b02..d5e1814172 100644 --- a/Kernel/FileSystem/Ext2FileSystem.cpp +++ b/Kernel/FileSystem/Ext2FileSystem.cpp @@ -807,7 +807,7 @@ RefPtr Ext2FS::get_inode(InodeIdentifier inode) const return new_inode; } -ssize_t Ext2FSInode::read_bytes(off_t offset, ssize_t count, UserOrKernelBuffer& buffer, FileDescription* description) const +KResultOr Ext2FSInode::read_bytes(off_t offset, ssize_t count, UserOrKernelBuffer& buffer, FileDescription* description) const { Locker inode_locker(m_lock); VERIFY(offset >= 0); @@ -823,7 +823,7 @@ ssize_t Ext2FSInode::read_bytes(off_t offset, ssize_t count, UserOrKernelBuffer& VERIFY(offset == 0); ssize_t nread = min((off_t)size() - offset, static_cast(count)); if (!buffer.write(((const u8*)m_raw_inode.i_block) + offset, (size_t)nread)) - return -EFAULT; + return EFAULT; return nread; } @@ -832,7 +832,7 @@ ssize_t Ext2FSInode::read_bytes(off_t offset, ssize_t count, UserOrKernelBuffer& if (m_block_list.is_empty()) { dmesgln("Ext2FSInode[{}]::read_bytes(): Empty block list", identifier()); - return -EIO; + return EIO; } bool allow_cache = !description || !description->is_direct(); @@ -859,7 +859,7 @@ ssize_t Ext2FSInode::read_bytes(off_t offset, ssize_t count, UserOrKernelBuffer& if (block_index.value() == 0) { // This is a hole, act as if it's filled with zeroes. if (!buffer_offset.memset(0, num_bytes_to_copy)) - return -EFAULT; + return EFAULT; } else { if (auto result = fs().read_block(block_index, &buffer_offset, num_bytes_to_copy, offset_into_block, allow_cache); result.is_error()) { dmesgln("Ext2FSInode[{}]::read_bytes(): Failed to read block {} (index {})", identifier(), block_index.value(), bi); @@ -940,19 +940,19 @@ KResult Ext2FSInode::resize(u64 new_size) auto clear_from = old_size; u8 zero_buffer[PAGE_SIZE] {}; while (bytes_to_clear) { - auto nwritten = write_bytes(clear_from, min(static_cast(sizeof(zero_buffer)), bytes_to_clear), UserOrKernelBuffer::for_kernel_buffer(zero_buffer), nullptr); - if (nwritten < 0) - return KResult((ErrnoCode)-nwritten); - VERIFY(nwritten != 0); - bytes_to_clear -= nwritten; - clear_from += nwritten; + auto result = write_bytes(clear_from, min(static_cast(sizeof(zero_buffer)), bytes_to_clear), UserOrKernelBuffer::for_kernel_buffer(zero_buffer), nullptr); + if (result.is_error()) + return result.error(); + VERIFY(result.value() != 0); + bytes_to_clear -= result.value(); + clear_from += result.value(); } } return KSuccess; } -ssize_t Ext2FSInode::write_bytes(off_t offset, ssize_t count, const UserOrKernelBuffer& data, FileDescription* description) +KResultOr Ext2FSInode::write_bytes(off_t offset, ssize_t count, const UserOrKernelBuffer& data, FileDescription* description) { VERIFY(offset >= 0); VERIFY(count >= 0); @@ -967,7 +967,7 @@ ssize_t Ext2FSInode::write_bytes(off_t offset, ssize_t count, const UserOrKernel if (max((size_t)(offset + count), (size_t)m_raw_inode.i_size) < max_inline_symlink_length) { dbgln_if(EXT2_DEBUG, "Ext2FSInode[{}]::write_bytes(): Poking into i_block array for inline symlink '{}' ({} bytes)", identifier(), data.copy_into_string(count), count); if (!data.read(((u8*)m_raw_inode.i_block) + offset, (size_t)count)) - return -EFAULT; + return EFAULT; if ((size_t)(offset + count) > (size_t)m_raw_inode.i_size) m_raw_inode.i_size = offset + count; set_metadata_dirty(true); @@ -988,7 +988,7 @@ ssize_t Ext2FSInode::write_bytes(off_t offset, ssize_t count, const UserOrKernel if (m_block_list.is_empty()) { dbgln("Ext2FSInode[{}]::write_bytes(): Empty block list", identifier()); - return -EIO; + return EIO; } BlockBasedFS::BlockIndex first_block_logical_index = offset / block_size; @@ -1113,11 +1113,11 @@ KResult Ext2FSInode::write_directory(const Vector& entries stream.fill_to_end(0); auto buffer = UserOrKernelBuffer::for_kernel_buffer(stream.data()); - ssize_t nwritten = write_bytes(0, stream.size(), buffer, nullptr); - if (nwritten < 0) - return KResult((ErrnoCode)-nwritten); + auto result = write_bytes(0, stream.size(), buffer, nullptr); + if (result.is_error()) + return result.error(); set_metadata_dirty(true); - if (static_cast(nwritten) != directory_data.size()) + if (static_cast(result.value()) != directory_data.size()) return EIO; return KSuccess; } diff --git a/Kernel/FileSystem/Ext2FileSystem.h b/Kernel/FileSystem/Ext2FileSystem.h index 25b55c9f87..6e666a4137 100644 --- a/Kernel/FileSystem/Ext2FileSystem.h +++ b/Kernel/FileSystem/Ext2FileSystem.h @@ -38,12 +38,12 @@ public: private: // ^Inode - virtual ssize_t read_bytes(off_t, ssize_t, UserOrKernelBuffer& buffer, FileDescription*) const override; + virtual KResultOr read_bytes(off_t, ssize_t, UserOrKernelBuffer& buffer, FileDescription*) const override; virtual InodeMetadata metadata() 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 UserOrKernelBuffer& data, FileDescription*) override; + virtual KResultOr write_bytes(off_t, ssize_t, const UserOrKernelBuffer& data, FileDescription*) override; virtual KResultOr> create_child(const String& name, mode_t, dev_t, uid_t, gid_t) override; virtual KResult add_child(Inode& child, const StringView& name, mode_t) override; virtual KResult remove_child(const StringView& name) override; diff --git a/Kernel/FileSystem/Inode.cpp b/Kernel/FileSystem/Inode.cpp index 5872d2d919..2adef0aa6e 100644 --- a/Kernel/FileSystem/Inode.cpp +++ b/Kernel/FileSystem/Inode.cpp @@ -60,9 +60,10 @@ KResultOr> Inode::read_entire(FileDescription* descriptio off_t offset = 0; for (;;) { auto buf = UserOrKernelBuffer::for_kernel_buffer(buffer); - nread = read_bytes(offset, sizeof(buffer), buf, description); - if (nread < 0) - return KResult((ErrnoCode)-nread); + auto result = read_bytes(offset, sizeof(buffer), buf, description); + if (result.is_error()) + return result.error(); + nread = result.value(); VERIFY(nread <= (ssize_t)sizeof(buffer)); if (nread <= 0) break; diff --git a/Kernel/FileSystem/Inode.h b/Kernel/FileSystem/Inode.h index 8b64922c60..6eb78be34d 100644 --- a/Kernel/FileSystem/Inode.h +++ b/Kernel/FileSystem/Inode.h @@ -52,10 +52,10 @@ public: virtual KResult attach(FileDescription&) { return KSuccess; } virtual void detach(FileDescription&) { } virtual void did_seek(FileDescription&, off_t) { } - virtual ssize_t read_bytes(off_t, ssize_t, UserOrKernelBuffer& buffer, FileDescription*) const = 0; + virtual KResultOr read_bytes(off_t, ssize_t, UserOrKernelBuffer& buffer, FileDescription*) 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 UserOrKernelBuffer& data, FileDescription*) = 0; + virtual KResultOr write_bytes(off_t, ssize_t, const UserOrKernelBuffer& data, FileDescription*) = 0; virtual KResultOr> create_child(const String& name, mode_t, dev_t, uid_t, gid_t) = 0; virtual KResult add_child(Inode&, const StringView& name, mode_t) = 0; virtual KResult remove_child(const StringView& name) = 0; diff --git a/Kernel/FileSystem/InodeFile.cpp b/Kernel/FileSystem/InodeFile.cpp index 235cc0a1c9..7018aff72c 100644 --- a/Kernel/FileSystem/InodeFile.cpp +++ b/Kernel/FileSystem/InodeFile.cpp @@ -31,13 +31,14 @@ KResultOr InodeFile::read(FileDescription& description, u64 offset, User if (Checked::addition_would_overflow(offset, count)) return EOVERFLOW; - ssize_t nread = m_inode->read_bytes(offset, count, buffer, &description); + auto result = m_inode->read_bytes(offset, count, buffer, &description); + if (result.is_error()) + return result.error(); + auto nread = result.value(); if (nread > 0) { Thread::current()->did_file_read(nread); evaluate_block_conditions(); } - if (nread < 0) - return KResult((ErrnoCode)-nread); return nread; } @@ -46,7 +47,11 @@ KResultOr InodeFile::write(FileDescription& description, u64 offset, con if (Checked::addition_would_overflow(offset, count)) return EOVERFLOW; - ssize_t nwritten = m_inode->write_bytes(offset, count, data, &description); + auto result = m_inode->write_bytes(offset, count, data, &description); + if (result.is_error()) + return result.error(); + + auto nwritten = result.value(); if (nwritten > 0) { auto mtime_result = m_inode->set_mtime(kgettimeofday().to_truncated_seconds()); Thread::current()->did_file_write(nwritten); @@ -54,8 +59,6 @@ KResultOr InodeFile::write(FileDescription& description, u64 offset, con if (mtime_result.is_error()) return mtime_result; } - if (nwritten < 0) - return KResult((ErrnoCode)-nwritten); return nwritten; } diff --git a/Kernel/FileSystem/Plan9FileSystem.cpp b/Kernel/FileSystem/Plan9FileSystem.cpp index 163dc3cb3c..6049e14e6a 100644 --- a/Kernel/FileSystem/Plan9FileSystem.cpp +++ b/Kernel/FileSystem/Plan9FileSystem.cpp @@ -728,7 +728,7 @@ KResult Plan9FSInode::ensure_open_for_mode(int mode) } } -ssize_t Plan9FSInode::read_bytes(off_t offset, ssize_t size, UserOrKernelBuffer& buffer, FileDescription*) const +KResultOr Plan9FSInode::read_bytes(off_t offset, ssize_t size, UserOrKernelBuffer& buffer, FileDescription*) const { auto result = const_cast(*this).ensure_open_for_mode(O_RDONLY); if (result.is_error()) @@ -762,22 +762,22 @@ ssize_t Plan9FSInode::read_bytes(off_t offset, ssize_t size, UserOrKernelBuffer& // Guard against the server returning more data than requested. size_t nread = min(data.length(), (size_t)size); if (!buffer.write(data.characters_without_null_termination(), nread)) - return -EFAULT; + return EFAULT; return nread; } -ssize_t Plan9FSInode::write_bytes(off_t offset, ssize_t size, const UserOrKernelBuffer& data, FileDescription*) +KResultOr Plan9FSInode::write_bytes(off_t offset, ssize_t size, const UserOrKernelBuffer& data, FileDescription*) { auto result = ensure_open_for_mode(O_WRONLY); if (result.is_error()) - return result; + return result.error(); size = fs().adjust_buffer_size(size); auto data_copy = data.copy_into_string(size); // FIXME: this seems ugly if (data_copy.is_null()) - return -EFAULT; + return EFAULT; Plan9FS::Message message { fs(), Plan9FS::Message::Type::Twrite }; message << fid() << (u64)offset; diff --git a/Kernel/FileSystem/Plan9FileSystem.h b/Kernel/FileSystem/Plan9FileSystem.h index 71a64b1062..dd654e668b 100644 --- a/Kernel/FileSystem/Plan9FileSystem.h +++ b/Kernel/FileSystem/Plan9FileSystem.h @@ -156,8 +156,8 @@ public: // ^Inode virtual InodeMetadata metadata() const override; virtual void flush_metadata() override; - virtual ssize_t read_bytes(off_t, ssize_t, UserOrKernelBuffer& buffer, FileDescription*) const override; - virtual ssize_t write_bytes(off_t, ssize_t, const UserOrKernelBuffer& data, FileDescription*) override; + virtual KResultOr read_bytes(off_t, ssize_t, UserOrKernelBuffer& buffer, FileDescription*) const override; + virtual KResultOr write_bytes(off_t, ssize_t, const UserOrKernelBuffer& data, FileDescription*) 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; diff --git a/Kernel/FileSystem/ProcFS.cpp b/Kernel/FileSystem/ProcFS.cpp index 01c439ad12..9abde39287 100644 --- a/Kernel/FileSystem/ProcFS.cpp +++ b/Kernel/FileSystem/ProcFS.cpp @@ -1222,17 +1222,17 @@ InodeMetadata ProcFSInode::metadata() const return metadata; } -ssize_t ProcFSInode::read_bytes(off_t offset, ssize_t count, UserOrKernelBuffer& buffer, FileDescription* description) const +KResultOr ProcFSInode::read_bytes(off_t offset, ssize_t count, UserOrKernelBuffer& buffer, FileDescription* description) const { dbgln_if(PROCFS_DEBUG, "ProcFS: read_bytes offset: {} count: {}", offset, count); VERIFY(offset >= 0); VERIFY(buffer.user_or_kernel_ptr()); if (!description) - return -EIO; + return EIO; if (!description->data()) { dbgln_if(PROCFS_DEBUG, "ProcFS: Do not have cached data!"); - return -EIO; + return EIO; } // Be sure to keep a reference to data_buffer while we use it! @@ -1243,7 +1243,7 @@ ssize_t ProcFSInode::read_bytes(off_t offset, ssize_t count, UserOrKernelBuffer& ssize_t nread = min(static_cast(data_buffer->size() - offset), static_cast(count)); if (!buffer.write(data_buffer->data() + offset, nread)) - return -EFAULT; + return EFAULT; return nread; } @@ -1454,7 +1454,7 @@ void ProcFSInode::flush_metadata() { } -ssize_t ProcFSInode::write_bytes(off_t offset, ssize_t size, const UserOrKernelBuffer& buffer, FileDescription*) +KResultOr ProcFSInode::write_bytes(off_t offset, ssize_t size, const UserOrKernelBuffer& buffer, FileDescription*) { // For process-specific inodes, hold the process's ptrace lock across the write // and refuse to write at all data if the process is not dumpable. @@ -1493,10 +1493,10 @@ ssize_t ProcFSInode::write_bytes(off_t offset, ssize_t size, const UserOrKernelB break; } } else - return -EPERM; + return EPERM; } else { if (!directory_entry->write_callback) - return -EPERM; + return EPERM; write_callback = directory_entry->write_callback; } diff --git a/Kernel/FileSystem/ProcFS.h b/Kernel/FileSystem/ProcFS.h index 1b8b1b833b..8e8237a3ae 100644 --- a/Kernel/FileSystem/ProcFS.h +++ b/Kernel/FileSystem/ProcFS.h @@ -78,12 +78,12 @@ private: // ^Inode virtual KResult attach(FileDescription&) override; virtual void did_seek(FileDescription&, off_t) override; - virtual ssize_t read_bytes(off_t, ssize_t, UserOrKernelBuffer& buffer, FileDescription*) const override; + virtual KResultOr read_bytes(off_t, ssize_t, UserOrKernelBuffer& buffer, FileDescription*) const override; virtual InodeMetadata metadata() 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 UserOrKernelBuffer& buffer, FileDescription*) override; + virtual KResultOr write_bytes(off_t, ssize_t, const UserOrKernelBuffer& buffer, FileDescription*) 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; virtual KResult remove_child(const StringView& name) override; @@ -111,12 +111,12 @@ private: // ^Inode virtual KResult attach(FileDescription&) override; virtual void did_seek(FileDescription&, off_t) override; - virtual ssize_t read_bytes(off_t, ssize_t, UserOrKernelBuffer&, FileDescription*) const override { VERIFY_NOT_REACHED(); } + virtual KResultOr read_bytes(off_t, ssize_t, UserOrKernelBuffer&, FileDescription*) const override { VERIFY_NOT_REACHED(); } virtual InodeMetadata metadata() const override; virtual KResult traverse_as_directory(Function) const override { VERIFY_NOT_REACHED(); } virtual RefPtr lookup(StringView name) override; virtual void flush_metadata() override {}; - virtual ssize_t write_bytes(off_t, ssize_t, const UserOrKernelBuffer&, FileDescription*) override { VERIFY_NOT_REACHED(); } + virtual KResultOr write_bytes(off_t, ssize_t, const UserOrKernelBuffer&, FileDescription*) override { VERIFY_NOT_REACHED(); } 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; virtual KResult remove_child(const StringView& name) override; diff --git a/Kernel/FileSystem/TmpFS.cpp b/Kernel/FileSystem/TmpFS.cpp index a0ac7c3b22..541a1ad5a7 100644 --- a/Kernel/FileSystem/TmpFS.cpp +++ b/Kernel/FileSystem/TmpFS.cpp @@ -126,7 +126,7 @@ KResult TmpFSInode::traverse_as_directory(Function TmpFSInode::read_bytes(off_t offset, ssize_t size, UserOrKernelBuffer& buffer, FileDescription*) const { Locker locker(m_lock, Lock::Mode::Shared); VERIFY(!is_directory()); @@ -143,11 +143,11 @@ ssize_t TmpFSInode::read_bytes(off_t offset, ssize_t size, UserOrKernelBuffer& b size = m_metadata.size - offset; if (!buffer.write(m_content->data() + offset, size)) - return -EFAULT; + return EFAULT; return size; } -ssize_t TmpFSInode::write_bytes(off_t offset, ssize_t size, const UserOrKernelBuffer& buffer, FileDescription*) +KResultOr TmpFSInode::write_bytes(off_t offset, ssize_t size, const UserOrKernelBuffer& buffer, FileDescription*) { Locker locker(m_lock); VERIFY(!is_directory()); @@ -175,7 +175,7 @@ ssize_t TmpFSInode::write_bytes(off_t offset, ssize_t size, const UserOrKernelBu // existing ones. auto tmp = KBuffer::try_create_with_size(new_size * 2); if (!tmp) - return -ENOMEM; + return ENOMEM; tmp->set_size(new_size); if (m_content) memcpy(tmp->data(), m_content->data(), old_size); @@ -187,7 +187,7 @@ ssize_t TmpFSInode::write_bytes(off_t offset, ssize_t size, const UserOrKernelBu } if (!buffer.read(m_content->data() + offset, size)) // TODO: partial reads? - return -EFAULT; + return EFAULT; return size; } diff --git a/Kernel/FileSystem/TmpFS.h b/Kernel/FileSystem/TmpFS.h index ed6eefb29a..0557e00286 100644 --- a/Kernel/FileSystem/TmpFS.h +++ b/Kernel/FileSystem/TmpFS.h @@ -54,12 +54,12 @@ public: const TmpFS& fs() const { return static_cast(Inode::fs()); } // ^Inode - virtual ssize_t read_bytes(off_t, ssize_t, UserOrKernelBuffer& buffer, FileDescription*) const override; + virtual KResultOr read_bytes(off_t, ssize_t, UserOrKernelBuffer& buffer, FileDescription*) const override; virtual InodeMetadata metadata() 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 UserOrKernelBuffer& buffer, FileDescription*) override; + virtual KResultOr write_bytes(off_t, ssize_t, const UserOrKernelBuffer& buffer, FileDescription*) 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; virtual KResult remove_child(const StringView& name) override; diff --git a/Kernel/FileSystem/VirtualFileSystem.cpp b/Kernel/FileSystem/VirtualFileSystem.cpp index 7955e8c25c..f06203c2a9 100644 --- a/Kernel/FileSystem/VirtualFileSystem.cpp +++ b/Kernel/FileSystem/VirtualFileSystem.cpp @@ -710,9 +710,9 @@ KResult VFS::symlink(StringView target, StringView linkpath, Custody& base) return inode_or_error.error(); auto& inode = inode_or_error.value(); auto target_buffer = UserOrKernelBuffer::for_kernel_buffer(const_cast((const u8*)target.characters_without_null_termination())); - ssize_t nwritten = inode->write_bytes(0, target.length(), target_buffer, nullptr); - if (nwritten < 0) - return KResult((ErrnoCode)-nwritten); + auto result = inode->write_bytes(0, target.length(), target_buffer, nullptr); + if (result.is_error()) + return result.error(); return KSuccess; } diff --git a/Kernel/Syscalls/mmap.cpp b/Kernel/Syscalls/mmap.cpp index b8fee3101b..6c0f70f191 100644 --- a/Kernel/Syscalls/mmap.cpp +++ b/Kernel/Syscalls/mmap.cpp @@ -50,8 +50,8 @@ static bool should_make_executable_exception_for_dynamic_loader(bool make_readab Elf32_Ehdr header; auto buffer = UserOrKernelBuffer::for_kernel_buffer((u8*)&header); - auto nread = inode.read_bytes(0, sizeof(header), buffer, nullptr); - if (nread != sizeof(header)) + auto result = inode.read_bytes(0, sizeof(header), buffer, nullptr); + if (result.is_error() || result.value() != sizeof(header)) return false; // The file is a valid ELF binary diff --git a/Kernel/VM/Region.cpp b/Kernel/VM/Region.cpp index 53e3c6b2bd..92d6d67394 100644 --- a/Kernel/VM/Region.cpp +++ b/Kernel/VM/Region.cpp @@ -540,13 +540,14 @@ PageFaultResponse Region::handle_inode_fault(size_t page_index_in_region, Scoped // Reading the page may block, so release the MM lock temporarily mm_lock.unlock(); auto buffer = UserOrKernelBuffer::for_kernel_buffer(page_buffer); - auto nread = inode.read_bytes(page_index_in_vmobject * PAGE_SIZE, PAGE_SIZE, buffer, nullptr); + auto result = inode.read_bytes(page_index_in_vmobject * PAGE_SIZE, PAGE_SIZE, buffer, nullptr); mm_lock.lock(); - if (nread < 0) { - dmesgln("MM: handle_inode_fault had error ({}) while reading!", nread); + if (result.is_error()) { + dmesgln("MM: handle_inode_fault had error ({}) while reading!", result.error()); return PageFaultResponse::ShouldCrash; } + auto nread = result.value(); if (nread < PAGE_SIZE) { // If we read less than a page, zero out the rest to avoid leaking uninitialized data. memset(page_buffer + nread, 0, PAGE_SIZE - nread);