Browse Source

Kernel: Tidy up Memory::AddressSpace construction

- Return KResultOr<T> in places
- Propagate errors
- Use TRY()
Andreas Kling 3 years ago
parent
commit
83fed5b2de

+ 3 - 7
Kernel/Memory/AddressSpace.cpp

@@ -15,14 +15,10 @@
 
 namespace Kernel::Memory {
 
-OwnPtr<AddressSpace> AddressSpace::try_create(AddressSpace const* parent)
+KResultOr<NonnullOwnPtr<AddressSpace>> AddressSpace::try_create(AddressSpace const* parent)
 {
-    auto page_directory = PageDirectory::try_create_for_userspace(parent ? &parent->page_directory().range_allocator() : nullptr);
-    if (!page_directory)
-        return {};
-    auto space = adopt_own_if_nonnull(new (nothrow) AddressSpace(page_directory.release_nonnull()));
-    if (!space)
-        return {};
+    auto page_directory = TRY(PageDirectory::try_create_for_userspace(parent ? &parent->page_directory().range_allocator() : nullptr));
+    auto space = TRY(adopt_nonnull_own_or_enomem(new (nothrow) AddressSpace(page_directory)));
     space->page_directory().set_space({}, *space);
     return space;
 }

+ 1 - 1
Kernel/Memory/AddressSpace.h

@@ -18,7 +18,7 @@ namespace Kernel::Memory {
 
 class AddressSpace {
 public:
-    static OwnPtr<AddressSpace> try_create(AddressSpace const* parent);
+    static KResultOr<NonnullOwnPtr<AddressSpace>> try_create(AddressSpace const* parent);
     ~AddressSpace();
 
     PageDirectory& page_directory() { return *m_page_directory; }

+ 4 - 6
Kernel/Memory/PageDirectory.cpp

@@ -42,14 +42,12 @@ UNMAP_AFTER_INIT NonnullRefPtr<PageDirectory> PageDirectory::must_create_kernel_
     return directory;
 }
 
-RefPtr<PageDirectory> PageDirectory::try_create_for_userspace(VirtualRangeAllocator const* parent_range_allocator)
+KResultOr<NonnullRefPtr<PageDirectory>> PageDirectory::try_create_for_userspace(VirtualRangeAllocator const* parent_range_allocator)
 {
     constexpr FlatPtr userspace_range_base = 0x00800000;
     FlatPtr const userspace_range_ceiling = USER_RANGE_CEILING;
 
-    auto directory = adopt_ref_if_nonnull(new (nothrow) PageDirectory);
-    if (!directory)
-        return {};
+    auto directory = TRY(adopt_nonnull_ref_or_enomem(new (nothrow) PageDirectory));
 
     if (parent_range_allocator) {
         directory->m_range_allocator.initialize_from_parent(*parent_range_allocator);
@@ -70,12 +68,12 @@ RefPtr<PageDirectory> PageDirectory::try_create_for_userspace(VirtualRangeAlloca
 
     directory->m_directory_table = MM.allocate_user_physical_page();
     if (!directory->m_directory_table)
-        return {};
+        return ENOMEM;
     auto kernel_pd_index = (kernel_mapping_base >> 30) & 0x1ffu;
     for (size_t i = 0; i < kernel_pd_index; i++) {
         directory->m_directory_pages[i] = MM.allocate_user_physical_page();
         if (!directory->m_directory_pages[i])
-            return {};
+            return ENOMEM;
     }
 
     // Share the top 1 GiB of kernel-only mappings (>=kernel_mapping_base)

+ 1 - 1
Kernel/Memory/PageDirectory.h

@@ -19,7 +19,7 @@ class PageDirectory : public RefCounted<PageDirectory> {
     friend class MemoryManager;
 
 public:
-    static RefPtr<PageDirectory> try_create_for_userspace(VirtualRangeAllocator const* parent_range_allocator = nullptr);
+    static KResultOr<NonnullRefPtr<PageDirectory>> try_create_for_userspace(VirtualRangeAllocator const* parent_range_allocator = nullptr);
     static NonnullRefPtr<PageDirectory> must_create_kernel_page_directory();
     static RefPtr<PageDirectory> find_by_cr3(FlatPtr);
 

+ 4 - 10
Kernel/Process.cpp

@@ -219,16 +219,10 @@ void Process::unprotect_data()
 
 KResultOr<NonnullRefPtr<Process>> Process::try_create(RefPtr<Thread>& first_thread, String const& name, UserID uid, GroupID gid, ProcessID ppid, bool is_kernel_process, RefPtr<Custody> cwd, RefPtr<Custody> executable, TTY* tty, Process* fork_parent)
 {
-    auto space = Memory::AddressSpace::try_create(fork_parent ? &fork_parent->address_space() : nullptr);
-    if (!space)
-        return ENOMEM;
-    auto process = adopt_ref_if_nonnull(new (nothrow) Process(name, uid, gid, ppid, is_kernel_process, move(cwd), move(executable), tty));
-    if (!process)
-        return ENOMEM;
-    auto result = process->attach_resources(space.release_nonnull(), first_thread, fork_parent);
-    if (result.is_error())
-        return result;
-    return process.release_nonnull();
+    auto space = TRY(Memory::AddressSpace::try_create(fork_parent ? &fork_parent->address_space() : nullptr));
+    auto process = TRY(adopt_nonnull_ref_or_enomem(new (nothrow) Process(name, uid, gid, ppid, is_kernel_process, move(cwd), move(executable), tty)));
+    TRY(process->attach_resources(move(space), first_thread, fork_parent));
+    return process;
 }
 
 Process::Process(const String& name, UserID uid, GroupID gid, ProcessID ppid, bool is_kernel_process, RefPtr<Custody> cwd, RefPtr<Custody> executable, TTY* tty)

+ 3 - 5
Kernel/Syscalls/execve.cpp

@@ -453,9 +453,7 @@ static KResultOr<LoadResult> load_elf_object(NonnullOwnPtr<Memory::AddressSpace>
 KResultOr<LoadResult> Process::load(NonnullRefPtr<FileDescription> main_program_description,
     RefPtr<FileDescription> interpreter_description, const ElfW(Ehdr) & main_program_header)
 {
-    auto new_space = Memory::AddressSpace::try_create(nullptr);
-    if (!new_space)
-        return ENOMEM;
+    auto new_space = TRY(Memory::AddressSpace::try_create(nullptr));
 
     ScopeGuard space_guard([&]() {
         Memory::MemoryManager::enter_process_paging_scope(*this);
@@ -467,7 +465,7 @@ KResultOr<LoadResult> Process::load(NonnullRefPtr<FileDescription> main_program_
     }
 
     if (interpreter_description.is_null()) {
-        auto result = load_elf_object(new_space.release_nonnull(), main_program_description, load_offset.value(), ShouldAllocateTls::Yes, ShouldAllowSyscalls::No);
+        auto result = load_elf_object(move(new_space), main_program_description, load_offset.value(), ShouldAllocateTls::Yes, ShouldAllowSyscalls::No);
         if (result.is_error())
             return result.error();
 
@@ -478,7 +476,7 @@ KResultOr<LoadResult> Process::load(NonnullRefPtr<FileDescription> main_program_
         return result;
     }
 
-    auto interpreter_load_result = load_elf_object(new_space.release_nonnull(), *interpreter_description, load_offset.value(), ShouldAllocateTls::No, ShouldAllowSyscalls::Yes);
+    auto interpreter_load_result = load_elf_object(move(new_space), *interpreter_description, load_offset.value(), ShouldAllocateTls::No, ShouldAllowSyscalls::Yes);
 
     if (interpreter_load_result.is_error())
         return interpreter_load_result.error();