Kernel: Change Inode::{read/write}_bytes interface to KResultOr<ssize_t>

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<T>. This change switches the interface and then all
implementers to use the new style.

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<ssize_t> 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<ssize_t> 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<ssize_t> 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<ssize_t> DevFSLinkInode::write_bytes(off_t offset, ssize_t count, const UserOrKernelBuffer& buffer, FileDescription*)
 {
  Locker locker(m_lock);
  VERIFY(offset == 0);
@@ -345,15 +346,15 @@ 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<ssize_t> DevFSDeviceInode::read_bytes(off_t offset, ssize_t count, UserOrKernelBuffer& buffer, FileDescription* description) const
 {
  Locker locker(m_lock);
  VERIFY(!!description);
  if (!m_attached_device->can_read(*description, offset))
  return 0;
  auto nread = const_cast<Device&>(*m_attached_device).read(*description, offset, buffer, count);
  if (nread.is_error())
- return -EIO;
+ return EIO;
  return nread.value();
 }
 
@@ -371,15 +372,15 @@ 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<ssize_t> DevFSDeviceInode::write_bytes(off_t offset, ssize_t count, const UserOrKernelBuffer& buffer, FileDescription* description)
 {
  Locker locker(m_lock);
  VERIFY(!!description);
  if (!m_attached_device->can_write(*description, offset))
  return 0;
  auto nread = const_cast<Device&>(*m_attached_device).write(*description, offset, buffer, count);
  if (nread.is_error())
- return -EIO;
+ return EIO;
  return nread.value();
 }
 

Kernel/FileSystem/DevFS.h

@@ -57,11 +57,11 @@ class DevFSInode : public Inode {
 
 protected:
  DevFSInode(DevFS&);
- virtual ssize_t read_bytes(off_t, ssize_t, UserOrKernelBuffer& buffer, FileDescription*) const override;
+ virtual KResultOr<ssize_t> read_bytes(off_t, ssize_t, UserOrKernelBuffer& buffer, FileDescription*) const override;
  virtual KResult traverse_as_directory(Function<bool(const FS::DirectoryEntryView&)>) const override;
  virtual RefPtr<Inode> 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<ssize_t> write_bytes(off_t, ssize_t, const UserOrKernelBuffer& buffer, FileDescription*) override;
  virtual KResultOr<NonnullRefPtr<Inode>> 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 @@ class DevFSDeviceInode : public DevFSInode {
  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<ssize_t> 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<ssize_t> write_bytes(off_t, ssize_t, const UserOrKernelBuffer& buffer, FileDescription*) override;
  virtual KResult chown(uid_t, gid_t) override;
 
  NonnullRefPtr<Device> m_attached_device;
@@ -106,9 +106,9 @@ class DevFSLinkInode : public DevFSInode {
 protected:
  DevFSLinkInode(DevFS&, String);
  // ^Inode
- virtual ssize_t read_bytes(off_t, ssize_t, UserOrKernelBuffer& buffer, FileDescription*) const override;
+ virtual KResultOr<ssize_t> 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<ssize_t> write_bytes(off_t, ssize_t, const UserOrKernelBuffer& buffer, FileDescription*) override;
 
  const String m_name;
  String m_link;

Kernel/FileSystem/DevPtsFS.cpp

@@ -100,12 +100,12 @@ DevPtsFSInode::~DevPtsFSInode()
 {
 }
 
-ssize_t DevPtsFSInode::read_bytes(off_t, ssize_t, UserOrKernelBuffer&, FileDescription*) const
+KResultOr<ssize_t> 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<ssize_t> DevPtsFSInode::write_bytes(off_t, ssize_t, const UserOrKernelBuffer&, FileDescription*)
 {
  VERIFY_NOT_REACHED();
 }

Kernel/FileSystem/DevPtsFS.h

@@ -47,12 +47,12 @@ class DevPtsFSInode final : public Inode {
  DevPtsFSInode(DevPtsFS&, InodeIndex, SlavePTY*);
 
  // ^Inode
- virtual ssize_t read_bytes(off_t, ssize_t, UserOrKernelBuffer& buffer, FileDescription*) const override;
+ virtual KResultOr<ssize_t> read_bytes(off_t, ssize_t, UserOrKernelBuffer& buffer, FileDescription*) const override;
  virtual InodeMetadata metadata() const override;
  virtual KResult traverse_as_directory(Function<bool(const FS::DirectoryEntryView&)>) const override;
  virtual RefPtr<Inode> 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<ssize_t> write_bytes(off_t, ssize_t, const UserOrKernelBuffer& buffer, FileDescription*) override;
  virtual KResultOr<NonnullRefPtr<Inode>> 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;

Kernel/FileSystem/Ext2FileSystem.cpp

@@ -807,7 +807,7 @@ RefPtr<Inode> 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<ssize_t> 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<off_t>(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<u64>(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<u64>(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<ssize_t> 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<Ext2FSDirectoryEntry>& 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<size_t>(nwritten) != directory_data.size())
+ if (static_cast<size_t>(result.value()) != directory_data.size())
  return EIO;
  return KSuccess;
 }

Kernel/FileSystem/Ext2FileSystem.h

@@ -38,12 +38,12 @@ class Ext2FSInode final : public Inode {
 
 private:
  // ^Inode
- virtual ssize_t read_bytes(off_t, ssize_t, UserOrKernelBuffer& buffer, FileDescription*) const override;
+ virtual KResultOr<ssize_t> read_bytes(off_t, ssize_t, UserOrKernelBuffer& buffer, FileDescription*) const override;
  virtual InodeMetadata metadata() const override;
  virtual KResult traverse_as_directory(Function<bool(const FS::DirectoryEntryView&)>) const override;
  virtual RefPtr<Inode> 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<ssize_t> write_bytes(off_t, ssize_t, const UserOrKernelBuffer& data, FileDescription*) override;
  virtual KResultOr<NonnullRefPtr<Inode>> 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;

Kernel/FileSystem/Inode.cpp

@@ -60,9 +60,10 @@ KResultOr<NonnullOwnPtr<KBuffer>> 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;

Kernel/FileSystem/Inode.h

@@ -52,10 +52,10 @@ class Inode : public RefCounted<Inode>
  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<ssize_t> read_bytes(off_t, ssize_t, UserOrKernelBuffer& buffer, FileDescription*) const = 0;
  virtual KResult traverse_as_directory(Function<bool(const FS::DirectoryEntryView&)>) const = 0;
  virtual RefPtr<Inode> lookup(StringView name) = 0;
- virtual ssize_t write_bytes(off_t, ssize_t, const UserOrKernelBuffer& data, FileDescription*) = 0;
+ virtual KResultOr<ssize_t> write_bytes(off_t, ssize_t, const UserOrKernelBuffer& data, FileDescription*) = 0;
  virtual KResultOr<NonnullRefPtr<Inode>> 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;

Kernel/FileSystem/InodeFile.cpp

@@ -31,13 +31,14 @@ KResultOr<size_t> InodeFile::read(FileDescription& description, u64 offset, User
  if (Checked<off_t>::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,16 +47,18 @@ KResultOr<size_t> InodeFile::write(FileDescription& description, u64 offset, con
  if (Checked<off_t>::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);
  evaluate_block_conditions();
  if (mtime_result.is_error())
  return mtime_result;
  }
- if (nwritten < 0)
- return KResult((ErrnoCode)-nwritten);
  return nwritten;
 }
 

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<ssize_t> Plan9FSInode::read_bytes(off_t offset, ssize_t size, UserOrKernelBuffer& buffer, FileDescription*) const
 {
  auto result = const_cast<Plan9FSInode&>(*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<ssize_t> 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;

Kernel/FileSystem/Plan9FileSystem.h

@@ -156,8 +156,8 @@ class Plan9FSInode final : public Inode {
  // ^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<ssize_t> read_bytes(off_t, ssize_t, UserOrKernelBuffer& buffer, FileDescription*) const override;
+ virtual KResultOr<ssize_t> write_bytes(off_t, ssize_t, const UserOrKernelBuffer& data, FileDescription*) override;
  virtual KResult traverse_as_directory(Function<bool(const FS::DirectoryEntryView&)>) const override;
  virtual RefPtr<Inode> lookup(StringView name) override;
  virtual KResultOr<NonnullRefPtr<Inode>> create_child(const String& name, mode_t, dev_t, uid_t, gid_t) override;