From 8e79bde2b73d18f31e4dcc1b33c61cd5ec73c83d Mon Sep 17 00:00:00 2001 From: Andreas Kling Date: Fri, 18 Dec 2020 14:10:10 +0100 Subject: Kernel: Move KBufferBuilder to the fallible KBuffer API KBufferBuilder::build() now returns an OwnPtr and can fail. Clients of the API have been updated to handle that situation. --- Kernel/FileSystem/Ext2FileSystem.cpp | 2 +- Kernel/FileSystem/FileDescription.cpp | 2 +- Kernel/FileSystem/FileDescription.h | 6 +- Kernel/FileSystem/Inode.cpp | 9 ++- Kernel/FileSystem/Inode.h | 2 +- Kernel/FileSystem/Plan9FileSystem.cpp | 37 ++++++------ Kernel/FileSystem/ProcFS.cpp | 105 +++++++++++++++++----------------- Kernel/FileSystem/ProcFS.h | 4 +- 8 files changed, 88 insertions(+), 79 deletions(-) (limited to 'Kernel/FileSystem') diff --git a/Kernel/FileSystem/Ext2FileSystem.cpp b/Kernel/FileSystem/Ext2FileSystem.cpp index 41d0dcd39f..55e29eb294 100644 --- a/Kernel/FileSystem/Ext2FileSystem.cpp +++ b/Kernel/FileSystem/Ext2FileSystem.cpp @@ -928,7 +928,7 @@ KResult Ext2FSInode::traverse_as_directory(Function(buffer.data()); while (entry < buffer.end_pointer()) { diff --git a/Kernel/FileSystem/FileDescription.cpp b/Kernel/FileSystem/FileDescription.cpp index ced161052a..02958ed401 100644 --- a/Kernel/FileSystem/FileDescription.cpp +++ b/Kernel/FileSystem/FileDescription.cpp @@ -179,7 +179,7 @@ bool FileDescription::can_read() const return m_file->can_read(*this, offset()); } -KResultOr FileDescription::read_entire_file() +KResultOr> FileDescription::read_entire_file() { // HACK ALERT: (This entire function) ASSERT(m_file->is_inode()); diff --git a/Kernel/FileSystem/FileDescription.h b/Kernel/FileSystem/FileDescription.h index 03d281c12e..d4e9cb2d1d 100644 --- a/Kernel/FileSystem/FileDescription.h +++ b/Kernel/FileSystem/FileDescription.h @@ -73,7 +73,7 @@ public: ssize_t get_dir_entries(UserOrKernelBuffer& buffer, ssize_t); - KResultOr read_entire_file(); + KResultOr> read_entire_file(); String absolute_path() const; @@ -122,7 +122,7 @@ public: FIFO::Direction fifo_direction() { return m_fifo_direction; } void set_fifo_direction(Badge, FIFO::Direction direction) { m_fifo_direction = direction; } - Optional& generator_cache() { return m_generator_cache; } + OwnPtr& generator_cache() { return m_generator_cache; } void set_original_inode(Badge, NonnullRefPtr&& inode) { m_inode = move(inode); } @@ -150,7 +150,7 @@ private: off_t m_current_offset { 0 }; - Optional m_generator_cache; + OwnPtr m_generator_cache; u32 m_file_flags { 0 }; diff --git a/Kernel/FileSystem/Inode.cpp b/Kernel/FileSystem/Inode.cpp index 1cc0b8438a..2cd982a84a 100644 --- a/Kernel/FileSystem/Inode.cpp +++ b/Kernel/FileSystem/Inode.cpp @@ -71,7 +71,7 @@ void Inode::sync() } } -KResultOr Inode::read_entire(FileDescription* descriptor) const +KResultOr> Inode::read_entire(FileDescription* descriptor) const { KBufferBuilder builder; @@ -96,7 +96,10 @@ KResultOr Inode::read_entire(FileDescription* descriptor) const return KResult(nread); } - return builder.build(); + auto entire_file = builder.build(); + if (!entire_file) + return KResult(-ENOMEM); + return entire_file.release_nonnull(); } KResultOr> Inode::resolve_as_link(Custody& base, RefPtr* out_parent, int options, int symlink_recursion_level) const @@ -109,7 +112,7 @@ KResultOr> Inode::resolve_as_link(Custody& base, RefPtrdata(), contents->size()); return VFS::the().resolve_path(path, base, out_parent, options, symlink_recursion_level); } diff --git a/Kernel/FileSystem/Inode.h b/Kernel/FileSystem/Inode.h index 7cea0178c6..72a0fb23b6 100644 --- a/Kernel/FileSystem/Inode.h +++ b/Kernel/FileSystem/Inode.h @@ -67,7 +67,7 @@ public: InodeIdentifier identifier() const { return { fsid(), index() }; } virtual InodeMetadata metadata() const = 0; - KResultOr read_entire(FileDescription* = nullptr) const; + KResultOr> read_entire(FileDescription* = nullptr) const; virtual ssize_t read_bytes(off_t, ssize_t, UserOrKernelBuffer& buffer, FileDescription*) const = 0; virtual KResult traverse_as_directory(Function) const = 0; diff --git a/Kernel/FileSystem/Plan9FileSystem.cpp b/Kernel/FileSystem/Plan9FileSystem.cpp index 3f7f17dd52..b3ac7d137a 100644 --- a/Kernel/FileSystem/Plan9FileSystem.cpp +++ b/Kernel/FileSystem/Plan9FileSystem.cpp @@ -185,7 +185,7 @@ public: u16 tag() const { return m_tag; } Message(Plan9FS&, Type); - Message(KBuffer&&); + Message(NonnullOwnPtr&&); ~Message(); Message& operator=(Message&&); @@ -205,7 +205,7 @@ private: union { KBufferBuilder m_builder; struct { - KBuffer buffer; + NonnullOwnPtr buffer; Decoder decoder; } m_built; }; @@ -356,8 +356,8 @@ Plan9FS::Message::Message(Plan9FS& fs, Type type) *this << size_placeholder << (u8)type << m_tag; } -Plan9FS::Message::Message(KBuffer&& buffer) - : m_built { buffer, Decoder({ buffer.data(), buffer.size() }) } +Plan9FS::Message::Message(NonnullOwnPtr&& buffer) + : m_built { move(buffer), Decoder({ buffer->data(), buffer->size() }) } , m_have_been_built(true) { u32 size; @@ -369,7 +369,7 @@ Plan9FS::Message::Message(KBuffer&& buffer) Plan9FS::Message::~Message() { if (m_have_been_built) { - m_built.buffer.~KBuffer(); + m_built.buffer.~NonnullOwnPtr(); m_built.decoder.~Decoder(); } else { m_builder.~KBufferBuilder(); @@ -382,7 +382,7 @@ Plan9FS::Message& Plan9FS::Message::operator=(Message&& message) m_type = message.m_type; if (m_have_been_built) { - m_built.buffer.~KBuffer(); + m_built.buffer.~NonnullOwnPtr(); m_built.decoder.~Decoder(); } else { m_builder.~KBufferBuilder(); @@ -390,7 +390,7 @@ Plan9FS::Message& Plan9FS::Message::operator=(Message&& message) m_have_been_built = message.m_have_been_built; if (m_have_been_built) { - new (&m_built.buffer) KBuffer(move(message.m_built.buffer)); + new (&m_built.buffer) NonnullOwnPtr(move(message.m_built.buffer)); new (&m_built.decoder) Decoder(move(message.m_built.decoder)); } else { new (&m_builder) KBufferBuilder(move(message.m_builder)); @@ -405,14 +405,17 @@ const KBuffer& Plan9FS::Message::build() auto tmp_buffer = m_builder.build(); + // FIXME: We should not assume success here. + ASSERT(tmp_buffer); + m_have_been_built = true; m_builder.~KBufferBuilder(); - new (&m_built.buffer) KBuffer(move(tmp_buffer)); - new (&m_built.decoder) Decoder({ m_built.buffer.data(), m_built.buffer.size() }); - u32* size = reinterpret_cast(m_built.buffer.data()); - *size = m_built.buffer.size(); - return m_built.buffer; + new (&m_built.buffer) NonnullOwnPtr(tmp_buffer.release_nonnull()); + new (&m_built.decoder) Decoder({ m_built.buffer->data(), m_built.buffer->size() }); + u32* size = reinterpret_cast(m_built.buffer->data()); + *size = m_built.buffer->size(); + return *m_built.buffer; } Plan9FS::ReceiveCompletion::ReceiveCompletion(u16 tag) @@ -578,10 +581,12 @@ KResult Plan9FS::read_and_dispatch_one_message() if (result.is_error()) return result; - auto buffer = KBuffer::create_with_size(header.size, Region::Access::Read | Region::Access::Write); + auto buffer = KBuffer::try_create_with_size(header.size, Region::Access::Read | Region::Access::Write); + if (!buffer) + return KResult(-ENOMEM); // Copy the already read header into the buffer. - memcpy(buffer.data(), &header, sizeof(header)); - result = do_read(buffer.data() + sizeof(header), header.size - sizeof(header)); + memcpy(buffer->data(), &header, sizeof(header)); + result = do_read(buffer->data() + sizeof(header), header.size - sizeof(header)); if (result.is_error()) return result; @@ -592,7 +597,7 @@ KResult Plan9FS::read_and_dispatch_one_message() auto completion = optional_completion.value(); ScopedSpinLock lock(completion->lock); completion->result = KSuccess; - completion->message = new Message { move(buffer) }; + completion->message = new Message { buffer.release_nonnull() }; completion->completed = true; m_completions.remove(header.tag); diff --git a/Kernel/FileSystem/ProcFS.cpp b/Kernel/FileSystem/ProcFS.cpp index c81b41005a..9ea60115d2 100644 --- a/Kernel/FileSystem/ProcFS.cpp +++ b/Kernel/FileSystem/ProcFS.cpp @@ -262,7 +262,7 @@ ProcFS::~ProcFS() { } -static Optional procfs$pid_fds(InodeIdentifier identifier) +static OwnPtr procfs$pid_fds(InodeIdentifier identifier) { KBufferBuilder builder; JsonArraySerializer array { builder }; @@ -298,7 +298,7 @@ static Optional procfs$pid_fds(InodeIdentifier identifier) return builder.build(); } -static Optional procfs$pid_fd_entry(InodeIdentifier identifier) +static OwnPtr procfs$pid_fd_entry(InodeIdentifier identifier) { auto process = Process::from_pid(to_pid(identifier)); if (!process) @@ -307,10 +307,10 @@ static Optional procfs$pid_fd_entry(InodeIdentifier identifier) auto description = process->file_description(fd); if (!description) return {}; - return description->absolute_path().to_byte_buffer(); + return KBuffer::try_create_with_bytes(description->absolute_path().bytes()); } -static Optional procfs$pid_vm(InodeIdentifier identifier) +static OwnPtr procfs$pid_vm(InodeIdentifier identifier) { auto process = Process::from_pid(to_pid(identifier)); if (!process) @@ -359,7 +359,7 @@ static Optional procfs$pid_vm(InodeIdentifier identifier) return builder.build(); } -static Optional procfs$pci(InodeIdentifier) +static OwnPtr procfs$pci(InodeIdentifier) { KBufferBuilder builder; JsonArraySerializer array { builder }; @@ -381,7 +381,7 @@ static Optional procfs$pci(InodeIdentifier) return builder.build(); } -static Optional procfs$interrupts(InodeIdentifier) +static OwnPtr procfs$interrupts(InodeIdentifier) { KBufferBuilder builder; JsonArraySerializer array { builder }; @@ -398,7 +398,7 @@ static Optional procfs$interrupts(InodeIdentifier) return builder.build(); } -static Optional procfs$keymap(InodeIdentifier) +static OwnPtr procfs$keymap(InodeIdentifier) { KBufferBuilder builder; JsonObjectSerializer json { builder }; @@ -407,7 +407,7 @@ static Optional procfs$keymap(InodeIdentifier) return builder.build(); } -static Optional procfs$devices(InodeIdentifier) +static OwnPtr procfs$devices(InodeIdentifier) { KBufferBuilder builder; JsonArraySerializer array { builder }; @@ -428,14 +428,14 @@ static Optional procfs$devices(InodeIdentifier) return builder.build(); } -static Optional procfs$uptime(InodeIdentifier) +static OwnPtr procfs$uptime(InodeIdentifier) { KBufferBuilder builder; builder.appendf("%llu\n", TimeManagement::the().uptime_ms() / 1000); return builder.build(); } -static Optional procfs$cmdline(InodeIdentifier) +static OwnPtr procfs$cmdline(InodeIdentifier) { KBufferBuilder builder; builder.append(kernel_command_line().string()); @@ -443,7 +443,7 @@ static Optional procfs$cmdline(InodeIdentifier) return builder.build(); } -static Optional procfs$modules(InodeIdentifier) +static OwnPtr procfs$modules(InodeIdentifier) { extern HashMap>* g_modules; KBufferBuilder builder; @@ -463,7 +463,7 @@ static Optional procfs$modules(InodeIdentifier) return builder.build(); } -static Optional procfs$profile(InodeIdentifier) +static OwnPtr procfs$profile(InodeIdentifier) { InterruptDisabler disabler; KBufferBuilder builder; @@ -495,7 +495,7 @@ static Optional procfs$profile(InodeIdentifier) return builder.build(); } -static Optional procfs$net_adapters(InodeIdentifier) +static OwnPtr procfs$net_adapters(InodeIdentifier) { KBufferBuilder builder; JsonArraySerializer array { builder }; @@ -521,7 +521,7 @@ static Optional procfs$net_adapters(InodeIdentifier) return builder.build(); } -static Optional procfs$net_arp(InodeIdentifier) +static OwnPtr procfs$net_arp(InodeIdentifier) { KBufferBuilder builder; JsonArraySerializer array { builder }; @@ -535,7 +535,7 @@ static Optional procfs$net_arp(InodeIdentifier) return builder.build(); } -static Optional procfs$net_tcp(InodeIdentifier) +static OwnPtr procfs$net_tcp(InodeIdentifier) { KBufferBuilder builder; JsonArraySerializer array { builder }; @@ -557,7 +557,7 @@ static Optional procfs$net_tcp(InodeIdentifier) return builder.build(); } -static Optional procfs$net_udp(InodeIdentifier) +static OwnPtr procfs$net_udp(InodeIdentifier) { KBufferBuilder builder; JsonArraySerializer array { builder }; @@ -572,7 +572,7 @@ static Optional procfs$net_udp(InodeIdentifier) return builder.build(); } -static Optional procfs$net_local(InodeIdentifier) +static OwnPtr procfs$net_local(InodeIdentifier) { KBufferBuilder builder; JsonArraySerializer array { builder }; @@ -590,7 +590,7 @@ static Optional procfs$net_local(InodeIdentifier) return builder.build(); } -static Optional procfs$pid_vmobjects(InodeIdentifier identifier) +static OwnPtr procfs$pid_vmobjects(InodeIdentifier identifier) { auto process = Process::from_pid(to_pid(identifier)); if (!process) @@ -625,7 +625,7 @@ static Optional procfs$pid_vmobjects(InodeIdentifier identifier) return builder.build(); } -static Optional procfs$pid_unveil(InodeIdentifier identifier) +static OwnPtr procfs$pid_unveil(InodeIdentifier identifier) { auto process = Process::from_pid(to_pid(identifier)); if (!process) @@ -652,7 +652,7 @@ static Optional procfs$pid_unveil(InodeIdentifier identifier) return builder.build(); } -static Optional procfs$tid_stack(InodeIdentifier identifier) +static OwnPtr procfs$tid_stack(InodeIdentifier identifier) { auto thread = Thread::from_tid(to_tid(identifier)); if (!thread) @@ -663,40 +663,40 @@ static Optional procfs$tid_stack(InodeIdentifier identifier) return builder.build(); } -static Optional procfs$pid_exe(InodeIdentifier identifier) +static OwnPtr procfs$pid_exe(InodeIdentifier identifier) { auto process = Process::from_pid(to_pid(identifier)); if (!process) return {}; auto* custody = process->executable(); ASSERT(custody); - return custody->absolute_path().to_byte_buffer(); + return KBuffer::try_create_with_bytes(custody->absolute_path().bytes()); } -static Optional procfs$pid_cwd(InodeIdentifier identifier) +static OwnPtr procfs$pid_cwd(InodeIdentifier identifier) { auto process = Process::from_pid(to_pid(identifier)); if (!process) return {}; - return process->current_directory().absolute_path().to_byte_buffer(); + return KBuffer::try_create_with_bytes(process->current_directory().absolute_path().bytes()); } -static Optional procfs$pid_root(InodeIdentifier identifier) +static OwnPtr procfs$pid_root(InodeIdentifier identifier) { auto process = Process::from_pid(to_pid(identifier)); if (!process) return {}; - return process->root_directory_relative_to_global_root().absolute_path().to_byte_buffer(); + return KBuffer::try_create_with_bytes(process->root_directory_relative_to_global_root().absolute_path().to_byte_buffer()); } -static Optional procfs$self(InodeIdentifier) +static OwnPtr procfs$self(InodeIdentifier) { char buffer[16]; int written = snprintf(buffer, sizeof(buffer), "%d", Process::current()->pid().value()); - return KBuffer::copy((const u8*)buffer, static_cast(written)); + return KBuffer::try_create_with_bytes(ReadonlyBytes { buffer, static_cast(written) }); } -Optional procfs$mm(InodeIdentifier) +OwnPtr procfs$mm(InodeIdentifier) { InterruptDisabler disabler; KBufferBuilder builder; @@ -716,7 +716,7 @@ Optional procfs$mm(InodeIdentifier) return builder.build(); } -static Optional procfs$dmesg(InodeIdentifier) +static OwnPtr procfs$dmesg(InodeIdentifier) { InterruptDisabler disabler; KBufferBuilder builder; @@ -725,7 +725,7 @@ static Optional procfs$dmesg(InodeIdentifier) return builder.build(); } -static Optional procfs$mounts(InodeIdentifier) +static OwnPtr procfs$mounts(InodeIdentifier) { // FIXME: This is obviously racy against the VFS mounts changing. KBufferBuilder builder; @@ -744,7 +744,7 @@ static Optional procfs$mounts(InodeIdentifier) return builder.build(); } -static Optional procfs$df(InodeIdentifier) +static OwnPtr procfs$df(InodeIdentifier) { // FIXME: This is obviously racy against the VFS mounts changing. KBufferBuilder builder; @@ -771,7 +771,7 @@ static Optional procfs$df(InodeIdentifier) return builder.build(); } -static Optional procfs$cpuinfo(InodeIdentifier) +static OwnPtr procfs$cpuinfo(InodeIdentifier) { KBufferBuilder builder; JsonArraySerializer array { builder }; @@ -796,7 +796,7 @@ static Optional procfs$cpuinfo(InodeIdentifier) return builder.build(); } -Optional procfs$memstat(InodeIdentifier) +OwnPtr procfs$memstat(InodeIdentifier) { InterruptDisabler disabler; @@ -823,7 +823,7 @@ Optional procfs$memstat(InodeIdentifier) return builder.build(); } -static Optional procfs$all(InodeIdentifier) +static OwnPtr procfs$all(InodeIdentifier) { KBufferBuilder builder; JsonArraySerializer array { builder }; @@ -912,7 +912,7 @@ static Optional procfs$all(InodeIdentifier) return builder.build(); } -static Optional procfs$inodes(InodeIdentifier) +static OwnPtr procfs$inodes(InodeIdentifier) { KBufferBuilder builder; InterruptDisabler disabler; @@ -964,19 +964,19 @@ SysVariable& SysVariable::for_inode(InodeIdentifier id) return variable; } -static Optional read_sys_bool(InodeIdentifier inode_id) +static OwnPtr read_sys_bool(InodeIdentifier inode_id) { auto& variable = SysVariable::for_inode(inode_id); ASSERT(variable.type == SysVariable::Type::Boolean); - auto buffer = ByteBuffer::create_uninitialized(2); + u8 buffer[2]; auto* lockable_bool = reinterpret_cast*>(variable.address); { LOCKER(lockable_bool->lock(), Lock::Mode::Shared); buffer[0] = lockable_bool->resource() ? '1' : '0'; } buffer[1] = '\n'; - return buffer; + return KBuffer::try_create_with_bytes(ReadonlyBytes { buffer, sizeof(buffer) }); } static ssize_t write_sys_bool(InodeIdentifier inode_id, const UserOrKernelBuffer& buffer, size_t size) @@ -1008,14 +1008,14 @@ static ssize_t write_sys_bool(InodeIdentifier inode_id, const UserOrKernelBuffer return (ssize_t)size; } -static Optional read_sys_string(InodeIdentifier inode_id) +static OwnPtr read_sys_string(InodeIdentifier inode_id) { auto& variable = SysVariable::for_inode(inode_id); ASSERT(variable.type == SysVariable::Type::String); auto* lockable_string = reinterpret_cast*>(variable.address); LOCKER(lockable_string->lock(), Lock::Mode::Shared); - return lockable_string->resource().to_byte_buffer(); + return KBuffer::try_create_with_bytes(lockable_string->resource().bytes()); } static ssize_t write_sys_string(InodeIdentifier inode_id, const UserOrKernelBuffer& buffer, size_t size) @@ -1209,7 +1209,7 @@ ssize_t ProcFSInode::read_bytes(off_t offset, ssize_t count, UserOrKernelBuffer& auto* directory_entry = fs().get_directory_entry(identifier()); - Optional (*read_callback)(InodeIdentifier) = nullptr; + OwnPtr (*read_callback)(InodeIdentifier) = nullptr; if (directory_entry) read_callback = directory_entry->read_callback; else @@ -1238,26 +1238,27 @@ ssize_t ProcFSInode::read_bytes(off_t offset, ssize_t count, UserOrKernelBuffer& ASSERT(read_callback); - Optional generated_data; + OwnPtr descriptionless_generated_data; + KBuffer* data = nullptr; if (!description) { - generated_data = read_callback(identifier()); + descriptionless_generated_data = read_callback(identifier()); + data = descriptionless_generated_data.ptr(); } else { - if (!description->generator_cache().has_value()) + if (!description->generator_cache()) description->generator_cache() = (*read_callback)(identifier()); - generated_data = description->generator_cache(); + data = description->generator_cache().ptr(); } - auto& data = generated_data; - if (!data.has_value()) + if (!data) return 0; - if ((size_t)offset >= data.value().size()) + if ((size_t)offset >= data->size()) return 0; - ssize_t nread = min(static_cast(data.value().size() - offset), static_cast(count)); - if (!buffer.write(data.value().data() + offset, nread)) + ssize_t nread = min(static_cast(data->size() - offset), static_cast(count)); + if (!buffer.write(data->data() + offset, nread)) return -EFAULT; - if (nread == 0 && description && description->generator_cache().has_value()) + if (nread == 0 && description && description->generator_cache()) description->generator_cache().clear(); return nread; diff --git a/Kernel/FileSystem/ProcFS.h b/Kernel/FileSystem/ProcFS.h index 13ab6f14bc..f8d80ca45d 100644 --- a/Kernel/FileSystem/ProcFS.h +++ b/Kernel/FileSystem/ProcFS.h @@ -59,7 +59,7 @@ private: struct ProcFSDirectoryEntry { ProcFSDirectoryEntry() { } - ProcFSDirectoryEntry(const char* a_name, unsigned a_proc_file_type, bool a_supervisor_only, Optional (*read_callback)(InodeIdentifier) = nullptr, ssize_t (*write_callback)(InodeIdentifier, const UserOrKernelBuffer&, size_t) = nullptr, RefPtr&& a_inode = nullptr) + ProcFSDirectoryEntry(const char* a_name, unsigned a_proc_file_type, bool a_supervisor_only, OwnPtr (*read_callback)(InodeIdentifier) = nullptr, ssize_t (*write_callback)(InodeIdentifier, const UserOrKernelBuffer&, size_t) = nullptr, RefPtr&& a_inode = nullptr) : name(a_name) , proc_file_type(a_proc_file_type) , supervisor_only(a_supervisor_only) @@ -72,7 +72,7 @@ private: const char* name { nullptr }; unsigned proc_file_type { 0 }; bool supervisor_only { false }; - Optional (*read_callback)(InodeIdentifier); + OwnPtr (*read_callback)(InodeIdentifier); ssize_t (*write_callback)(InodeIdentifier, const UserOrKernelBuffer&, size_t); RefPtr inode; InodeIdentifier identifier(unsigned fsid) const; -- cgit v1.2.3