Kernel: Make KString factories return KResultOr + use TRY() everywhere

There are a number of places that don't have an error propagation path
right now, so I've added FIXME's about that.
This commit is contained in:
Andreas Kling 2021-09-06 19:24:54 +02:00
parent 69b9b2888c
commit 56a2594de7
Notes: sideshowbarker 2024-07-18 04:35:00 +09:00
21 changed files with 100 additions and 122 deletions

View File

@ -33,15 +33,10 @@ KResultOr<NonnullRefPtr<Custody>> Custody::try_create(Custody* parent, StringVie
}
}
auto name_kstring = KString::try_create(name);
if (!name_kstring)
return ENOMEM;
auto custody = adopt_ref_if_nonnull(new (nothrow) Custody(parent, name_kstring.release_nonnull(), inode, mount_flags));
if (!custody)
return ENOMEM;
auto name_kstring = TRY(KString::try_create(name));
auto custody = TRY(adopt_nonnull_ref_or_enomem(new (nothrow) Custody(parent, move(name_kstring), inode, mount_flags)));
all_custodies.prepend(*custody);
return custody.release_nonnull();
return custody;
});
}
@ -73,12 +68,8 @@ Custody::~Custody()
KResultOr<NonnullOwnPtr<KString>> Custody::try_serialize_absolute_path() const
{
if (!parent()) {
auto string = KString::try_create("/"sv);
if (!string)
return ENOMEM;
return string.release_nonnull();
}
if (!parent())
return KString::try_create("/"sv);
Vector<Custody const*, 32> custody_chain;
size_t path_length = 0;
@ -89,9 +80,7 @@ KResultOr<NonnullOwnPtr<KString>> Custody::try_serialize_absolute_path() const
VERIFY(path_length > 0);
char* buffer;
auto string = KString::try_create_uninitialized(path_length - 1, buffer);
if (!string)
return ENOMEM;
auto string = TRY(KString::try_create_uninitialized(path_length - 1, buffer));
size_t string_index = 0;
for (size_t custody_index = custody_chain.size() - 1; custody_index > 0; --custody_index) {
buffer[string_index] = '/';
@ -102,7 +91,7 @@ KResultOr<NonnullOwnPtr<KString>> Custody::try_serialize_absolute_path() const
}
VERIFY(string->length() == string_index);
buffer[string_index] = 0;
return string.release_nonnull();
return string;
}
String Custody::absolute_path() const

View File

@ -21,11 +21,10 @@ DevFS::DevFS()
void DevFS::notify_new_device(Device& device)
{
auto name = KString::try_create(device.device_name());
VERIFY(name);
// FIXME: Handle KString allocation failure.
auto name = KString::try_create(device.device_name()).release_value();
MutexLocker locker(m_lock);
auto new_device_inode = adopt_ref(*new DevFSDeviceInode(*this, device, name.release_nonnull()));
auto new_device_inode = adopt_ref(*new DevFSDeviceInode(*this, device, move(name)));
m_nodes.append(new_device_inode);
m_root_inode->m_devices.append(new_device_inode);
}
@ -276,10 +275,8 @@ KResultOr<NonnullRefPtr<Inode>> DevFSRootDirectoryInode::create_child(StringView
if (link.name() == name)
return EEXIST;
}
auto name_kstring = KString::try_create(name);
if (!name_kstring)
return ENOMEM;
auto new_link_inode = TRY(adopt_nonnull_ref_or_enomem(new (nothrow) DevFSLinkInode(fs(), name_kstring.release_nonnull())));
auto name_kstring = TRY(KString::try_create(name));
auto new_link_inode = TRY(adopt_nonnull_ref_or_enomem(new (nothrow) DevFSLinkInode(fs(), move(name_kstring))));
if (!m_links.try_ensure_capacity(m_links.size() + 1))
return ENOMEM;
if (!fs().m_nodes.try_ensure_capacity(fs().m_nodes.size() + 1))

View File

@ -21,7 +21,7 @@ static size_t allocate_inode_index()
}
SysFSComponent::SysFSComponent(StringView name)
: m_name(KString::try_create(name).release_nonnull())
: m_name(KString::try_create(name).release_value()) // FIXME: Handle KString allocation failure.
, m_component_index(allocate_inode_index())
{
}

View File

@ -282,11 +282,8 @@ KResult TmpFSInode::add_child(Inode& child, StringView const& name, mode_t)
if (name.length() > NAME_MAX)
return ENAMETOOLONG;
auto name_kstring = KString::try_create(name);
if (!name_kstring)
return ENOMEM;
auto* child_entry = new (nothrow) Child { name_kstring.release_nonnull(), static_cast<TmpFSInode&>(child) };
auto name_kstring = TRY(KString::try_create(name));
auto* child_entry = new (nothrow) Child { move(name_kstring), static_cast<TmpFSInode&>(child) };
if (!child_entry)
return ENOMEM;

View File

@ -323,9 +323,7 @@ KResultOr<NonnullRefPtr<FileDescription>> VirtualFileSystem::create(StringView p
{
auto basename = KLexicalPath::basename(path);
auto parent_path = TRY(parent_custody.try_serialize_absolute_path());
auto full_path = KLexicalPath::try_join(parent_path->view(), basename);
if (!full_path)
return ENOMEM;
auto full_path = TRY(KLexicalPath::try_join(parent_path->view(), basename));
TRY(validate_path_against_process_veil(full_path->view(), options));
if (!is_socket(mode) && !is_fifo(mode) && !is_block_device(mode) && !is_character_device(mode)) {

View File

@ -62,7 +62,7 @@ Vector<StringView> parts(StringView const& path)
return path.split_view('/');
}
OwnPtr<KString> try_join(StringView const& first, StringView const& second)
KResultOr<NonnullOwnPtr<KString>> try_join(StringView const& first, StringView const& second)
{
VERIFY(is_canonical(first));
VERIFY(is_canonical(second));
@ -70,24 +70,19 @@ OwnPtr<KString> try_join(StringView const& first, StringView const& second)
if (first == "/"sv) {
char* buffer;
auto string = KString::try_create_uninitialized(1 + second.length(), buffer);
if (!string)
return {};
auto string = TRY(KString::try_create_uninitialized(1 + second.length(), buffer));
buffer[0] = '/';
__builtin_memcpy(buffer + 1, second.characters_without_null_termination(), second.length());
buffer[string->length()] = 0;
return string;
} else {
char* buffer;
auto string = KString::try_create_uninitialized(first.length() + 1 + second.length(), buffer);
if (!string)
return string;
__builtin_memcpy(buffer, first.characters_without_null_termination(), first.length());
buffer[first.length()] = '/';
__builtin_memcpy(buffer + first.length() + 1, second.characters_without_null_termination(), second.length());
buffer[string->length()] = 0;
return string;
}
char* buffer;
auto string = TRY(KString::try_create_uninitialized(first.length() + 1 + second.length(), buffer));
__builtin_memcpy(buffer, first.characters_without_null_termination(), first.length());
buffer[first.length()] = '/';
__builtin_memcpy(buffer + first.length() + 1, second.characters_without_null_termination(), second.length());
buffer[string->length()] = 0;
return string;
}
}

View File

@ -17,6 +17,6 @@ StringView basename(StringView const&);
StringView dirname(StringView const&);
Vector<StringView> parts(StringView const&);
OwnPtr<KString> try_join(StringView const&, StringView const&);
KResultOr<NonnullOwnPtr<KString>> try_join(StringView const&, StringView const&);
}

View File

@ -10,49 +10,43 @@ extern bool g_in_early_boot;
namespace Kernel {
OwnPtr<KString> KString::try_create(StringView const& string)
KResultOr<NonnullOwnPtr<KString>> KString::try_create(StringView string)
{
char* characters = nullptr;
size_t length = string.length();
auto new_string = KString::try_create_uninitialized(length, characters);
if (!new_string)
return {};
auto new_string = TRY(KString::try_create_uninitialized(length, characters));
if (!string.is_empty())
__builtin_memcpy(characters, string.characters_without_null_termination(), length);
characters[length] = '\0';
return new_string;
}
NonnullOwnPtr<KString> KString::must_create(StringView const& string)
NonnullOwnPtr<KString> KString::must_create(StringView string)
{
// We can only enforce success during early boot.
VERIFY(g_in_early_boot);
auto kstring = KString::try_create(string);
VERIFY(kstring != nullptr);
return kstring.release_nonnull();
return KString::try_create(string).release_value();
}
OwnPtr<KString> KString::try_create_uninitialized(size_t length, char*& characters)
KResultOr<NonnullOwnPtr<KString>> KString::try_create_uninitialized(size_t length, char*& characters)
{
size_t allocation_size = sizeof(KString) + (sizeof(char) * length) + sizeof(char);
auto* slot = kmalloc(allocation_size);
if (!slot)
return {};
auto* new_string = new (slot) KString(length);
return ENOMEM;
auto new_string = TRY(adopt_nonnull_own_or_enomem(new (slot) KString(length)));
characters = new_string->m_characters;
return adopt_own_if_nonnull(new_string);
return new_string;
}
NonnullOwnPtr<KString> KString::must_create_uninitialized(size_t length, char*& characters)
{
// We can only enforce success during early boot.
VERIFY(g_in_early_boot);
auto kstring = KString::try_create_uninitialized(length, characters);
VERIFY(kstring != nullptr);
return kstring.release_nonnull();
return KString::try_create_uninitialized(length, characters).release_value();
}
OwnPtr<KString> KString::try_clone() const
KResultOr<NonnullOwnPtr<KString>> KString::try_clone() const
{
return try_create(view());
}

View File

@ -16,14 +16,14 @@ class KString {
AK_MAKE_NONMOVABLE(KString);
public:
[[nodiscard]] static OwnPtr<KString> try_create_uninitialized(size_t, char*&);
[[nodiscard]] static KResultOr<NonnullOwnPtr<KString>> try_create_uninitialized(size_t, char*&);
[[nodiscard]] static NonnullOwnPtr<KString> must_create_uninitialized(size_t, char*&);
[[nodiscard]] static OwnPtr<KString> try_create(StringView const&);
[[nodiscard]] static NonnullOwnPtr<KString> must_create(StringView const&);
[[nodiscard]] static KResultOr<NonnullOwnPtr<KString>> try_create(StringView);
[[nodiscard]] static NonnullOwnPtr<KString> must_create(StringView);
void operator delete(void*);
[[nodiscard]] OwnPtr<KString> try_clone() const;
[[nodiscard]] KResultOr<NonnullOwnPtr<KString>> try_clone() const;
[[nodiscard]] bool is_empty() const { return m_length == 0; }
[[nodiscard]] size_t length() const { return m_length; }

View File

@ -140,8 +140,12 @@ KResultOr<VirtualRange> AddressSpace::try_allocate_range(VirtualAddress vaddr, s
KResultOr<Region*> AddressSpace::try_allocate_split_region(Region const& source_region, VirtualRange const& range, size_t offset_in_vmobject)
{
OwnPtr<KString> region_name;
if (!source_region.name().is_null())
region_name = TRY(KString::try_create(source_region.name()));
auto new_region = TRY(Region::try_create_user_accessible(
range, source_region.vmobject(), offset_in_vmobject, KString::try_create(source_region.name()), source_region.access(), source_region.is_cacheable() ? Region::Cacheable::Yes : Region::Cacheable::No, source_region.is_shared()));
range, source_region.vmobject(), offset_in_vmobject, move(region_name), source_region.access(), source_region.is_cacheable() ? Region::Cacheable::Yes : Region::Cacheable::No, source_region.is_shared()));
auto* region = TRY(add_region(move(new_region)));
region->set_syscall_region(source_region.is_syscall_region());
region->set_mmap(source_region.is_mmap());
@ -157,8 +161,11 @@ KResultOr<Region*> AddressSpace::try_allocate_split_region(Region const& source_
KResultOr<Region*> AddressSpace::allocate_region(VirtualRange const& range, StringView name, int prot, AllocationStrategy strategy)
{
VERIFY(range.is_valid());
OwnPtr<KString> region_name;
if (!name.is_null())
region_name = TRY(KString::try_create(name));
auto vmobject = TRY(AnonymousVMObject::try_create_with_size(range.size(), strategy));
auto region = TRY(Region::try_create_user_accessible(range, move(vmobject), 0, KString::try_create(name), prot_to_region_access_flags(prot), Region::Cacheable::Yes, false));
auto region = TRY(Region::try_create_user_accessible(range, move(vmobject), 0, move(region_name), prot_to_region_access_flags(prot), Region::Cacheable::Yes, false));
TRY(region->map(page_directory()));
return add_region(move(region));
}
@ -180,7 +187,10 @@ KResultOr<Region*> AddressSpace::allocate_region_with_vmobject(VirtualRange cons
return EINVAL;
}
offset_in_vmobject &= PAGE_MASK;
auto region = TRY(Region::try_create_user_accessible(range, move(vmobject), offset_in_vmobject, KString::try_create(name), prot_to_region_access_flags(prot), Region::Cacheable::Yes, shared));
OwnPtr<KString> region_name;
if (!name.is_null())
region_name = TRY(KString::try_create(name));
auto region = TRY(Region::try_create_user_accessible(range, move(vmobject), offset_in_vmobject, move(region_name), prot_to_region_access_flags(prot), Region::Cacheable::Yes, shared));
auto* added_region = TRY(add_region(move(region)));
TRY(added_region->map(page_directory()));
return added_region;

View File

@ -729,7 +729,10 @@ KResultOr<NonnullOwnPtr<Region>> MemoryManager::allocate_kernel_region(PhysicalA
KResultOr<NonnullOwnPtr<Region>> MemoryManager::allocate_kernel_region_with_vmobject(VirtualRange const& range, VMObject& vmobject, StringView name, Region::Access access, Region::Cacheable cacheable)
{
auto region = TRY(Region::try_create_kernel_only(range, vmobject, 0, KString::try_create(name), access, cacheable));
OwnPtr<KString> name_kstring;
if (!name.is_null())
name_kstring = TRY(KString::try_create(name));
auto region = TRY(Region::try_create_kernel_only(range, vmobject, 0, move(name_kstring), access, cacheable));
TRY(region->map(kernel_page_directory()));
return region;
}

View File

@ -60,8 +60,13 @@ KResultOr<NonnullOwnPtr<Region>> Region::try_clone()
VERIFY(vmobject().is_shared_inode());
// Create a new region backed by the same VMObject.
OwnPtr<KString> region_name;
if (m_name)
region_name = TRY(m_name->try_clone());
auto region = TRY(Region::try_create_user_accessible(
m_range, m_vmobject, m_offset_in_vmobject, m_name ? m_name->try_clone() : OwnPtr<KString> {}, access(), m_cacheable ? Cacheable::Yes : Cacheable::No, m_shared));
m_range, m_vmobject, m_offset_in_vmobject, move(region_name), access(), m_cacheable ? Cacheable::Yes : Cacheable::No, m_shared));
region->set_mmap(m_mmap);
region->set_shared(m_shared);
region->set_syscall_region(is_syscall_region());
@ -75,8 +80,13 @@ KResultOr<NonnullOwnPtr<Region>> Region::try_clone()
// Set up a COW region. The parent (this) region becomes COW as well!
remap();
OwnPtr<KString> clone_region_name;
if (m_name)
clone_region_name = TRY(m_name->try_clone());
auto clone_region = TRY(Region::try_create_user_accessible(
m_range, vmobject_clone, m_offset_in_vmobject, m_name ? m_name->try_clone() : OwnPtr<KString> {}, access(), m_cacheable ? Cacheable::Yes : Cacheable::No, m_shared));
m_range, vmobject_clone, m_offset_in_vmobject, move(clone_region_name), access(), m_cacheable ? Cacheable::Yes : Cacheable::No, m_shared));
if (m_stack) {
VERIFY(is_readable());

View File

@ -127,9 +127,10 @@ KResult LocalSocket::bind(Userspace<const sockaddr*> user_address, socklen_t add
if (address.sun_family != AF_LOCAL)
return set_so_error(EINVAL);
auto path = KString::try_create(StringView { address.sun_path, strnlen(address.sun_path, sizeof(address.sun_path)) });
if (!path)
return set_so_error(ENOMEM);
auto path_kstring_or_error = KString::try_create(StringView { address.sun_path, strnlen(address.sun_path, sizeof(address.sun_path)) });
if (path_kstring_or_error.is_error())
return set_so_error(path_kstring_or_error.error());
auto path = path_kstring_or_error.release_value();
dbgln_if(LOCAL_SOCKET_DEBUG, "LocalSocket({}) bind({})", this, path);
@ -176,9 +177,10 @@ KResult LocalSocket::connect(FileDescription& description, Userspace<const socka
if (copy_from_user(&safe_address[0], &local_address.sun_path[0], sizeof(safe_address) - 1).is_error())
return set_so_error(EFAULT);
safe_address[sizeof(safe_address) - 1] = '\0';
maybe_path = KString::try_create(safe_address);
if (!maybe_path)
return set_so_error(ENOMEM);
auto path_kstring_or_error = KString::try_create(safe_address);
if (path_kstring_or_error.is_error())
return set_so_error(path_kstring_or_error.error());
maybe_path = path_kstring_or_error.release_value();
}
auto path = maybe_path.release_nonnull();
@ -517,10 +519,7 @@ KResultOr<NonnullRefPtr<FileDescription>> LocalSocket::recvfd(const FileDescript
KResult LocalSocket::try_set_path(StringView path)
{
auto kstring = KString::try_create(path);
if (!kstring)
return ENOMEM;
m_path = move(kstring);
m_path = TRY(KString::try_create(path));
return KSuccess;
}

View File

@ -864,11 +864,9 @@ KResult Process::set_coredump_property(NonnullOwnPtr<KString> key, NonnullOwnPtr
KResult Process::try_set_coredump_property(StringView key, StringView value)
{
auto key_kstring = KString::try_create(key);
auto value_kstring = KString::try_create(value);
if (key_kstring && value_kstring)
return set_coredump_property(key_kstring.release_nonnull(), value_kstring.release_nonnull());
return ENOMEM;
auto key_kstring = TRY(KString::try_create(key));
auto value_kstring = TRY(KString::try_create(value));
return set_coredump_property(move(key_kstring), move(value_kstring));
};
static constexpr StringView to_string(Pledge promise)

View File

@ -86,7 +86,10 @@ ProcFSExposedComponent::ProcFSExposedComponent()
ProcFSExposedComponent::ProcFSExposedComponent(StringView name)
: m_component_index(s_allocate_global_inode_index())
{
m_name = KString::try_create(name);
auto name_or_error = KString::try_create(name);
if (name_or_error.is_error())
TODO();
m_name = name_or_error.release_value();
}
ProcFSExposedDirectory::ProcFSExposedDirectory(StringView name)

View File

@ -26,20 +26,18 @@ Kernel::KResultOr<NonnullOwnPtr<Kernel::KString>> try_copy_kstring_from_user(Use
return EFAULT;
}
char* buffer;
auto new_string = Kernel::KString::try_create_uninitialized(length, buffer);
if (!new_string)
return ENOMEM;
auto new_string = TRY(Kernel::KString::try_create_uninitialized(length, buffer));
buffer[length] = '\0';
if (length == 0)
return new_string.release_nonnull();
return new_string;
if (!Kernel::safe_memcpy(buffer, user_str.unsafe_userspace_ptr(), (size_t)length, fault_at)) {
dbgln("copy_kstring_from_user({:p}, {}) failed at {} (memcpy)", static_cast<const void*>(user_str.unsafe_userspace_ptr()), user_str_size, VirtualAddress { fault_at });
return EFAULT;
}
return new_string.release_nonnull();
return new_string;
}
[[nodiscard]] Optional<Time> copy_time_from_user(const timespec* ts_user)

View File

@ -475,9 +475,7 @@ KResult Process::do_exec(NonnullRefPtr<FileDescription> main_program_description
return ENOENT;
auto new_process_name = parts.take_last();
auto new_main_thread_name = KString::try_create(new_process_name);
if (!new_main_thread_name)
return ENOMEM;
auto new_main_thread_name = TRY(KString::try_create(new_process_name));
auto main_program_metadata = main_program_description->metadata();
@ -598,7 +596,7 @@ KResult Process::do_exec(NonnullRefPtr<FileDescription> main_program_description
// NOTE: Be careful to not trigger any page faults below!
m_name = move(new_process_name);
new_main_thread->set_name(new_main_thread_name.release_nonnull());
new_main_thread->set_name(move(new_main_thread_name));
{
ProtectedDataMutationScope scope { *this };

View File

@ -44,15 +44,13 @@ KResultOr<FlatPtr> Process::sys$create_thread(void* (*entry)(void*), Userspace<c
auto thread = TRY(Thread::try_create(*this));
// FIXME: Don't make a temporary String here
auto new_thread_name = KString::try_create(String::formatted("{} [{}]", m_name, thread->tid().value()));
if (!new_thread_name)
return ENOMEM;
auto new_thread_name = TRY(KString::try_create(String::formatted("{} [{}]", m_name, thread->tid().value())));
// We know this thread is not the main_thread,
// So give it a unique name until the user calls $set_thread_name on it
// length + 4 to give space for our extra junk at the end
StringBuilder builder(m_name.length() + 4);
thread->set_name(new_thread_name.release_nonnull());
thread->set_name(move(new_thread_name));
if (!is_thread_joinable)
thread->detach();

View File

@ -86,9 +86,7 @@ KResultOr<FlatPtr> Process::sys$unveil(Userspace<const Syscall::SC_unveil_params
new_unveiled_path = TRY(custody_or_error.value()->try_serialize_absolute_path());
} else if (custody_or_error.error() == ENOENT && parent_custody && (new_permissions & UnveilAccess::CreateOrRemove)) {
auto parent_custody_path = TRY(parent_custody->try_serialize_absolute_path());
new_unveiled_path = KLexicalPath::try_join(parent_custody_path->view(), KLexicalPath::basename(path->view()));
if (!new_unveiled_path)
return ENOMEM;
new_unveiled_path = TRY(KLexicalPath::try_join(parent_custody_path->view(), KLexicalPath::basename(path->view())));
} else {
// FIXME Should this be EINVAL?
return custody_or_error.error();

View File

@ -46,11 +46,8 @@ KResultOr<NonnullRefPtr<Thread>> Thread::try_create(NonnullRefPtr<Process> proce
if (!block_timer)
return ENOMEM;
auto name = KString::try_create(process->name());
if (!name)
return ENOMEM;
return adopt_nonnull_ref_or_enomem(new (nothrow) Thread(move(process), move(kernel_stack_region), block_timer.release_nonnull(), name.release_nonnull()));
auto name = TRY(KString::try_create(process->name()));
return adopt_nonnull_ref_or_enomem(new (nothrow) Thread(move(process), move(kernel_stack_region), block_timer.release_nonnull(), move(name)));
}
Thread::Thread(NonnullRefPtr<Process> process, NonnullOwnPtr<Memory::Region> kernel_stack_region, NonnullRefPtr<Timer> block_timer, NonnullOwnPtr<KString> name)
@ -70,7 +67,8 @@ Thread::Thread(NonnullRefPtr<Process> process, NonnullOwnPtr<Memory::Region> ker
{
// FIXME: Go directly to KString
auto string = String::formatted("Kernel stack (thread {})", m_tid.value());
m_kernel_stack_region->set_name(KString::try_create(string));
// FIXME: Handle KString allocation failure.
m_kernel_stack_region->set_name(KString::try_create(string).release_value());
}
Thread::all_instances().with([&](auto& list) {

View File

@ -21,17 +21,12 @@ KResultOr<NonnullOwnPtr<KString>> UserOrKernelBuffer::try_copy_into_kstring(size
return EINVAL;
if (Memory::is_user_address(VirtualAddress(m_buffer))) {
char* buffer;
auto kstring = KString::try_create_uninitialized(size, buffer);
if (!kstring)
return ENOMEM;
auto kstring = TRY(KString::try_create_uninitialized(size, buffer));
TRY(copy_from_user(buffer, m_buffer, size));
return kstring.release_nonnull();
return kstring;
}
auto kstring = KString::try_create(ReadonlyBytes { m_buffer, size });
if (!kstring)
return ENOMEM;
return kstring.release_nonnull();
return KString::try_create(ReadonlyBytes { m_buffer, size });
}
bool UserOrKernelBuffer::write(const void* src, size_t offset, size_t len)