1
Fork 0
mirror of https://github.com/RGBCube/serenity synced 2025-07-25 05:07:34 +00:00

Kernel: Plumb KResult through FileDescription::read_entire_file() implementation.

Allow file system implementation to return meaningful error codes to
callers of the FileDescription::read_entire_file(). This allows both
Process::sys$readlink() and Process::sys$module_load() to return more
detailed errors to the user.
This commit is contained in:
Brian Gianforcaro 2020-05-26 00:36:11 -07:00 committed by Andreas Kling
parent c459e4ecb2
commit 6a74af8063
14 changed files with 54 additions and 36 deletions

View file

@ -146,10 +146,10 @@ InodeMetadata DevPtsFSInode::metadata() const
return m_metadata; return m_metadata;
} }
bool DevPtsFSInode::traverse_as_directory(Function<bool(const FS::DirectoryEntry&)> callback) const KResult DevPtsFSInode::traverse_as_directory(Function<bool(const FS::DirectoryEntry&)> callback) const
{ {
if (identifier().index() > 1) if (identifier().index() > 1)
return false; return KResult(-ENOTDIR);
callback({ ".", 1, identifier(), 0 }); callback({ ".", 1, identifier(), 0 });
callback({ "..", 2, identifier(), 0 }); callback({ "..", 2, identifier(), 0 });
@ -160,7 +160,7 @@ bool DevPtsFSInode::traverse_as_directory(Function<bool(const FS::DirectoryEntry
callback({ name.characters(), name.length(), identifier, 0 }); callback({ name.characters(), name.length(), identifier, 0 });
} }
return true; return KSuccess;
} }
size_t DevPtsFSInode::directory_entry_count() const size_t DevPtsFSInode::directory_entry_count() const

View file

@ -69,7 +69,7 @@ private:
// ^Inode // ^Inode
virtual ssize_t read_bytes(off_t, ssize_t, u8* buffer, FileDescription*) const override; virtual ssize_t read_bytes(off_t, ssize_t, u8* buffer, FileDescription*) const override;
virtual InodeMetadata metadata() const override; virtual InodeMetadata metadata() const override;
virtual bool traverse_as_directory(Function<bool(const FS::DirectoryEntry&)>) const override; virtual KResult traverse_as_directory(Function<bool(const FS::DirectoryEntry&)>) const override;
virtual RefPtr<Inode> lookup(StringView name) override; virtual RefPtr<Inode> lookup(StringView name) override;
virtual void flush_metadata() override; virtual void flush_metadata() override;
virtual ssize_t write_bytes(off_t, ssize_t, const u8* buffer, FileDescription*) override; virtual ssize_t write_bytes(off_t, ssize_t, const u8* buffer, FileDescription*) override;

View file

@ -849,7 +849,7 @@ ssize_t Ext2FSInode::write_bytes(off_t offset, ssize_t count, const u8* data, Fi
return nwritten; return nwritten;
} }
bool Ext2FSInode::traverse_as_directory(Function<bool(const FS::DirectoryEntry&)> callback) const KResult Ext2FSInode::traverse_as_directory(Function<bool(const FS::DirectoryEntry&)> callback) const
{ {
LOCKER(m_lock); LOCKER(m_lock);
ASSERT(is_directory()); ASSERT(is_directory());
@ -858,8 +858,12 @@ bool Ext2FSInode::traverse_as_directory(Function<bool(const FS::DirectoryEntry&)
dbg() << "Ext2FS: Traversing as directory: " << identifier(); dbg() << "Ext2FS: Traversing as directory: " << identifier();
#endif #endif
auto buffer = read_entire(); auto buffer_or = read_entire();
ASSERT(buffer); ASSERT(!buffer_or.is_error());
if (buffer_or.is_error())
return buffer_or.error();
auto buffer = buffer_or.value();
auto* entry = reinterpret_cast<ext2_dir_entry_2*>(buffer.data()); auto* entry = reinterpret_cast<ext2_dir_entry_2*>(buffer.data());
while (entry < buffer.end_pointer()) { while (entry < buffer.end_pointer()) {
@ -872,7 +876,8 @@ bool Ext2FSInode::traverse_as_directory(Function<bool(const FS::DirectoryEntry&)
} }
entry = (ext2_dir_entry_2*)((char*)entry + entry->rec_len); entry = (ext2_dir_entry_2*)((char*)entry + entry->rec_len);
} }
return true;
return KSuccess;
} }
bool Ext2FSInode::write_directory(const Vector<FS::DirectoryEntry>& entries) bool Ext2FSInode::write_directory(const Vector<FS::DirectoryEntry>& entries)
@ -944,7 +949,7 @@ KResult Ext2FSInode::add_child(InodeIdentifier child_id, const StringView& name,
Vector<FS::DirectoryEntry> entries; Vector<FS::DirectoryEntry> entries;
bool name_already_exists = false; bool name_already_exists = false;
traverse_as_directory([&](auto& entry) { KResult result = traverse_as_directory([&](auto& entry) {
if (name == entry.name) { if (name == entry.name) {
name_already_exists = true; name_already_exists = true;
return false; return false;
@ -952,6 +957,10 @@ KResult Ext2FSInode::add_child(InodeIdentifier child_id, const StringView& name,
entries.append(entry); entries.append(entry);
return true; return true;
}); });
if (result.is_error())
return result;
if (name_already_exists) { if (name_already_exists) {
dbg() << "Ext2FSInode::add_child(): Name '" << name << "' already exists in inode " << index(); dbg() << "Ext2FSInode::add_child(): Name '" << name << "' already exists in inode " << index();
return KResult(-EEXIST); return KResult(-EEXIST);
@ -959,7 +968,7 @@ KResult Ext2FSInode::add_child(InodeIdentifier child_id, const StringView& name,
auto child_inode = fs().get_inode(child_id); auto child_inode = fs().get_inode(child_id);
if (child_inode) { if (child_inode) {
auto result = child_inode->increment_link_count(); result = child_inode->increment_link_count();
if (result.is_error()) if (result.is_error())
return result; return result;
} }
@ -991,11 +1000,13 @@ KResult Ext2FSInode::remove_child(const StringView& name)
#endif #endif
Vector<FS::DirectoryEntry> entries; Vector<FS::DirectoryEntry> entries;
traverse_as_directory([&](auto& entry) { KResult result = traverse_as_directory([&](auto& entry) {
if (name != entry.name) if (name != entry.name)
entries.append(entry); entries.append(entry);
return true; return true;
}); });
if (result.is_error())
return result;
bool success = write_directory(entries); bool success = write_directory(entries);
if (!success) { if (!success) {

View file

@ -59,7 +59,7 @@ private:
// ^Inode // ^Inode
virtual ssize_t read_bytes(off_t, ssize_t, u8* buffer, FileDescription*) const override; virtual ssize_t read_bytes(off_t, ssize_t, u8* buffer, FileDescription*) const override;
virtual InodeMetadata metadata() const override; virtual InodeMetadata metadata() const override;
virtual bool traverse_as_directory(Function<bool(const FS::DirectoryEntry&)>) const override; virtual KResult traverse_as_directory(Function<bool(const FS::DirectoryEntry&)>) const override;
virtual RefPtr<Inode> lookup(StringView name) override; virtual RefPtr<Inode> lookup(StringView name) override;
virtual void flush_metadata() override; virtual void flush_metadata() override;
virtual ssize_t write_bytes(off_t, ssize_t, const u8* data, FileDescription*) override; virtual ssize_t write_bytes(off_t, ssize_t, const u8* data, FileDescription*) override;

View file

@ -158,7 +158,7 @@ bool FileDescription::can_read() const
return m_file->can_read(*this, offset()); return m_file->can_read(*this, offset());
} }
ByteBuffer FileDescription::read_entire_file() KResultOr<ByteBuffer> FileDescription::read_entire_file()
{ {
// HACK ALERT: (This entire function) // HACK ALERT: (This entire function)
ASSERT(m_file->is_inode()); ASSERT(m_file->is_inode());

View file

@ -79,7 +79,7 @@ public:
ssize_t get_dir_entries(u8* buffer, ssize_t); ssize_t get_dir_entries(u8* buffer, ssize_t);
ByteBuffer read_entire_file(); KResultOr<ByteBuffer> read_entire_file();
String absolute_path() const; String absolute_path() const;

View file

@ -61,7 +61,7 @@ void Inode::sync()
} }
} }
ByteBuffer Inode::read_entire(FileDescription* descriptor) const KResultOr<ByteBuffer> Inode::read_entire(FileDescription* descriptor) const
{ {
size_t initial_size = metadata().size ? metadata().size : 4096; size_t initial_size = metadata().size ? metadata().size : 4096;
StringBuilder builder(initial_size); StringBuilder builder(initial_size);
@ -92,8 +92,11 @@ KResultOr<NonnullRefPtr<Custody>> Inode::resolve_as_link(Custody& base, RefPtr<C
// The default implementation simply treats the stored // The default implementation simply treats the stored
// contents as a path and resolves that. That is, it // contents as a path and resolves that. That is, it
// behaves exactly how you would expect a symlink to work. // behaves exactly how you would expect a symlink to work.
auto contents = read_entire(); auto contents_or = read_entire();
if (contents_or.is_error())
return contents_or.error();
auto& contents = contents_or.value();
if (!contents) { if (!contents) {
if (out_parent) if (out_parent)
*out_parent = nullptr; *out_parent = nullptr;

View file

@ -66,10 +66,10 @@ public:
InodeIdentifier identifier() const { return { fsid(), index() }; } InodeIdentifier identifier() const { return { fsid(), index() }; }
virtual InodeMetadata metadata() const = 0; virtual InodeMetadata metadata() const = 0;
ByteBuffer read_entire(FileDescription* = nullptr) const; KResultOr<ByteBuffer> read_entire(FileDescription* = nullptr) const;
virtual ssize_t read_bytes(off_t, ssize_t, u8* buffer, FileDescription*) const = 0; virtual ssize_t read_bytes(off_t, ssize_t, u8* buffer, FileDescription*) const = 0;
virtual bool traverse_as_directory(Function<bool(const FS::DirectoryEntry&)>) const = 0; virtual KResult traverse_as_directory(Function<bool(const FS::DirectoryEntry&)>) const = 0;
virtual RefPtr<Inode> lookup(StringView name) = 0; virtual RefPtr<Inode> lookup(StringView name) = 0;
virtual ssize_t write_bytes(off_t, ssize_t, const u8* data, FileDescription*) = 0; virtual ssize_t write_bytes(off_t, ssize_t, const u8* data, FileDescription*) = 0;
virtual KResult add_child(InodeIdentifier child_id, const StringView& name, mode_t) = 0; virtual KResult add_child(InodeIdentifier child_id, const StringView& name, mode_t) = 0;

View file

@ -1253,14 +1253,14 @@ InodeIdentifier ProcFS::ProcFSDirectoryEntry::identifier(unsigned fsid) const
return to_identifier(fsid, PDI_Root, 0, (ProcFileType)proc_file_type); return to_identifier(fsid, PDI_Root, 0, (ProcFileType)proc_file_type);
} }
bool ProcFSInode::traverse_as_directory(Function<bool(const FS::DirectoryEntry&)> callback) const KResult ProcFSInode::traverse_as_directory(Function<bool(const FS::DirectoryEntry&)> callback) const
{ {
#ifdef PROCFS_DEBUG #ifdef PROCFS_DEBUG
dbg() << "ProcFS: traverse_as_directory " << index(); dbg() << "ProcFS: traverse_as_directory " << index();
#endif #endif
if (!Kernel::is_directory(identifier())) if (!Kernel::is_directory(identifier()))
return false; return KResult(-ENOTDIR);
auto pid = to_pid(identifier()); auto pid = to_pid(identifier());
auto proc_file_type = to_proc_file_type(identifier()); auto proc_file_type = to_proc_file_type(identifier());
@ -1303,7 +1303,7 @@ bool ProcFSInode::traverse_as_directory(Function<bool(const FS::DirectoryEntry&)
case FI_PID: { case FI_PID: {
auto handle = ProcessInspectionHandle::from_pid(pid); auto handle = ProcessInspectionHandle::from_pid(pid);
if (!handle) if (!handle)
return false; return KResult(-ENOENT);
auto& process = handle->process(); auto& process = handle->process();
for (auto& entry : fs().m_entries) { for (auto& entry : fs().m_entries) {
if (entry.proc_file_type > __FI_PID_Start && entry.proc_file_type < __FI_PID_End) { if (entry.proc_file_type > __FI_PID_Start && entry.proc_file_type < __FI_PID_End) {
@ -1318,7 +1318,7 @@ bool ProcFSInode::traverse_as_directory(Function<bool(const FS::DirectoryEntry&)
case FI_PID_fd: { case FI_PID_fd: {
auto handle = ProcessInspectionHandle::from_pid(pid); auto handle = ProcessInspectionHandle::from_pid(pid);
if (!handle) if (!handle)
return false; return KResult(-ENOENT);
auto& process = handle->process(); auto& process = handle->process();
for (int i = 0; i < process.max_open_file_descriptors(); ++i) { for (int i = 0; i < process.max_open_file_descriptors(); ++i) {
auto description = process.file_description(i); auto description = process.file_description(i);
@ -1330,10 +1330,10 @@ bool ProcFSInode::traverse_as_directory(Function<bool(const FS::DirectoryEntry&)
} }
} break; } break;
default: default:
return true; return KSuccess;
} }
return true; return KSuccess;
} }
RefPtr<Inode> ProcFSInode::lookup(StringView name) RefPtr<Inode> ProcFSInode::lookup(StringView name)

View file

@ -101,7 +101,7 @@ private:
// ^Inode // ^Inode
virtual ssize_t read_bytes(off_t, ssize_t, u8* buffer, FileDescription*) const override; virtual ssize_t read_bytes(off_t, ssize_t, u8* buffer, FileDescription*) const override;
virtual InodeMetadata metadata() const override; virtual InodeMetadata metadata() const override;
virtual bool traverse_as_directory(Function<bool(const FS::DirectoryEntry&)>) const override; virtual KResult traverse_as_directory(Function<bool(const FS::DirectoryEntry&)>) const override;
virtual RefPtr<Inode> lookup(StringView name) override; virtual RefPtr<Inode> lookup(StringView name) override;
virtual void flush_metadata() override; virtual void flush_metadata() override;
virtual ssize_t write_bytes(off_t, ssize_t, const u8* buffer, FileDescription*) override; virtual ssize_t write_bytes(off_t, ssize_t, const u8* buffer, FileDescription*) override;
@ -127,7 +127,7 @@ private:
// ^Inode // ^Inode
virtual ssize_t read_bytes(off_t, ssize_t, u8*, FileDescription*) const override { ASSERT_NOT_REACHED(); } virtual ssize_t read_bytes(off_t, ssize_t, u8*, FileDescription*) const override { ASSERT_NOT_REACHED(); }
virtual InodeMetadata metadata() const override; virtual InodeMetadata metadata() const override;
virtual bool traverse_as_directory(Function<bool(const FS::DirectoryEntry&)>) const override { ASSERT_NOT_REACHED(); } virtual KResult traverse_as_directory(Function<bool(const FS::DirectoryEntry&)>) const override { ASSERT_NOT_REACHED(); }
virtual RefPtr<Inode> lookup(StringView name) override; virtual RefPtr<Inode> lookup(StringView name) override;
virtual void flush_metadata() override {}; virtual void flush_metadata() override {};
virtual ssize_t write_bytes(off_t, ssize_t, const u8*, FileDescription*) override { ASSERT_NOT_REACHED(); } virtual ssize_t write_bytes(off_t, ssize_t, const u8*, FileDescription*) override { ASSERT_NOT_REACHED(); }

View file

@ -166,19 +166,19 @@ InodeMetadata TmpFSInode::metadata() const
return m_metadata; return m_metadata;
} }
bool TmpFSInode::traverse_as_directory(Function<bool(const FS::DirectoryEntry&)> callback) const KResult TmpFSInode::traverse_as_directory(Function<bool(const FS::DirectoryEntry&)> callback) const
{ {
LOCKER(m_lock, Lock::Mode::Shared); LOCKER(m_lock, Lock::Mode::Shared);
if (!is_directory()) if (!is_directory())
return false; return KResult(-ENOTDIR);
callback({ ".", identifier(), 0 }); callback({ ".", identifier(), 0 });
callback({ "..", m_parent, 0 }); callback({ "..", m_parent, 0 });
for (auto& it : m_children) for (auto& it : m_children)
callback(it.value.entry); callback(it.value.entry);
return true; return KSuccess;
} }
ssize_t TmpFSInode::read_bytes(off_t offset, ssize_t size, u8* buffer, FileDescription*) const ssize_t TmpFSInode::read_bytes(off_t offset, ssize_t size, u8* buffer, FileDescription*) const

View file

@ -79,7 +79,7 @@ public:
// ^Inode // ^Inode
virtual ssize_t read_bytes(off_t, ssize_t, u8* buffer, FileDescription*) const override; virtual ssize_t read_bytes(off_t, ssize_t, u8* buffer, FileDescription*) const override;
virtual InodeMetadata metadata() const override; virtual InodeMetadata metadata() const override;
virtual bool traverse_as_directory(Function<bool(const FS::DirectoryEntry&)>) const override; virtual KResult traverse_as_directory(Function<bool(const FS::DirectoryEntry&)>) const override;
virtual RefPtr<Inode> lookup(StringView name) override; virtual RefPtr<Inode> lookup(StringView name) override;
virtual void flush_metadata() override; virtual void flush_metadata() override;
virtual ssize_t write_bytes(off_t, ssize_t, const u8* buffer, FileDescription*) override; virtual ssize_t write_bytes(off_t, ssize_t, const u8* buffer, FileDescription*) override;

View file

@ -194,8 +194,8 @@ void load_kernel_symbol_table()
ASSERT(!result.is_error()); ASSERT(!result.is_error());
auto description = result.value(); auto description = result.value();
auto buffer = description->read_entire_file(); auto buffer = description->read_entire_file();
ASSERT(buffer); ASSERT(!buffer.is_error());
load_kernel_sybols_from_data(buffer); load_kernel_sybols_from_data(buffer.value());
} }
} }

View file

@ -1944,10 +1944,10 @@ int Process::sys$readlink(const Syscall::SC_readlink_params* user_params)
return -EINVAL; return -EINVAL;
auto contents = description->read_entire_file(); auto contents = description->read_entire_file();
if (!contents) if (!contents.is_error())
return -EIO; // FIXME: Get a more detailed error from VFS. return contents.error();
auto link_target = String::copy(contents); auto link_target = String::copy(contents.value());
if (link_target.length() > params.buffer.size) if (link_target.length() > params.buffer.size)
return -ENAMETOOLONG; return -ENAMETOOLONG;
copy_to_user(params.buffer.data, link_target.characters(), link_target.length()); copy_to_user(params.buffer.data, link_target.characters(), link_target.length());
@ -4444,7 +4444,11 @@ int Process::sys$module_load(const char* user_path, size_t path_length)
if (description_or_error.is_error()) if (description_or_error.is_error())
return description_or_error.error(); return description_or_error.error();
auto& description = description_or_error.value(); auto& description = description_or_error.value();
auto payload = description->read_entire_file(); auto payload_or_error = description->read_entire_file();
if (payload_or_error.is_error())
return payload_or_error.error();
auto payload = payload_or_error.value();
auto storage = KBuffer::create_with_size(payload.size()); auto storage = KBuffer::create_with_size(payload.size());
memcpy(storage.data(), payload.data(), payload.size()); memcpy(storage.data(), payload.data(), payload.size());
payload.clear(); payload.clear();