Heap: Rewrite kmalloc to use Option<HeapBlock*> instead of nullable pointers to iterate over the heap

At some point, this should be done inside LinkedList itself, but we have no such thing as break in for_each().
It's iterate over everything or nothing.

This also requires operator= in Option, might be also added to Result in the future.

kernel/src/boot/Init.cpp

@@ -15,8 +15,7 @@ void Init::check_magic()
     if (memcmp(bootboot.magic, BOOTBOOT_MAGIC, 4))
     {
         kerrorln("ERROR: Invalid magic value from bootloader");
-        for (;;)
+        CPU::efficient_halt();
-            ;
     }
 }
 

kernel/src/main.cpp

@@ -5,19 +5,15 @@
 #include "arch/MMU.h"
 #include "arch/Timer.h"
 #include "boot/Init.h"
-#include "boot/bootboot.h"
 #include "config.h"
 #include "memory/Heap.h"
 #include "memory/KernelVM.h"
 #include "memory/MemoryManager.h"
 #include "thread/Scheduler.h"
 #include <luna/CString.h>
-#include <luna/CircularQueue.h>
 #include <luna/Result.h>
 #include <luna/Units.h>
 
-extern const BOOTBOOT bootboot;
-
 void heap_thread()
 {
     CPU::disable_interrupts();

kernel/src/memory/Heap.cpp

@@ -199,45 +199,50 @@ Result<void*> kmalloc(usize size, bool should_scrub)
         heap.append(block);
     }
 
-    HeapBlock* block = heap.expect_first();
+    Option<HeapBlock*> block = heap.first();
-    while (block)
+    while (block.has_value())
     {
+        HeapBlock* const current = block.value();
         // Trying to find a free block...
-        if (is_block_free(block))
+        if (is_block_free(current))
         {
-            if (block->full_size < size)
+            if (current->full_size < size)
             {
-                block = heap.next(block).value_or(nullptr);
+                block = heap.next(current);
                 continue;
             }
             break; // We found a free block that's big enough!!
         }
-        auto rc = split(block, size);
+        auto rc = split(current, size);
         if (rc.has_value())
         {
             block = rc.value(); // We managed to get a free block from a larger used block!!
             break;
         }
-        block = heap.next(block).value_or(nullptr);
+        block = heap.next(current);
     }
 
-    if (!block) // No free blocks, let's allocate a new one
+    if (!block.has_value()) // No free blocks, let's allocate a new one
     {
         usize pages = get_pages_for_allocation(size + sizeof(HeapBlock));
-        block = TRY(allocate_pages(pages));
+        HeapBlock* const current = TRY(allocate_pages(pages));
 
-        block->full_size = (pages * ARCH_PAGE_SIZE) - sizeof(HeapBlock);
+        current->full_size = (pages * ARCH_PAGE_SIZE) - sizeof(HeapBlock);
-        block->magic = BLOCK_MAGIC;
+        current->magic = BLOCK_MAGIC;
-        block->status = BLOCK_START_MEM | BLOCK_END_MEM;
+        current->status = BLOCK_START_MEM | BLOCK_END_MEM;
-        heap.append(block);
+        heap.append(current);
+
+        block = current;
     }
 
-    block->req_size = size;
+    HeapBlock* const current = block.value();
-    block->status |= BLOCK_USED;
 
-    if (should_scrub) { memset(get_pointer_from_heap_block(block), KMALLOC_SCRUB_BYTE, size); }
+    current->req_size = size;
+    current->status |= BLOCK_USED;
 
-    return get_pointer_from_heap_block(block);
+    if (should_scrub) { memset(get_pointer_from_heap_block(current), KMALLOC_SCRUB_BYTE, size); }
+
+    return get_pointer_from_heap_block(current);
 }
 
 Result<void> kfree(void* ptr)
@@ -361,24 +366,25 @@ void dump_heap_usage()
     }
     usize alloc_total = 0;
     usize alloc_used = 0;
-    HeapBlock* block = heap.expect_first();
+    auto block = heap.first();
-    while (block)
+    while (block.has_value())
     {
-        if (is_block_free(block))
+        HeapBlock* current = block.value();
+        if (is_block_free(current))
         {
-            kdbgln("- Available block (%p), of size %zu (%s%s)", (void*)block, block->full_size,
+            kdbgln("- Available block (%p), of size %zu (%s%s)", (void*)current, current->full_size,
-                   block->status & BLOCK_START_MEM ? "b" : "-", block->status & BLOCK_END_MEM ? "e" : "-");
+                   current->status & BLOCK_START_MEM ? "b" : "-", current->status & BLOCK_END_MEM ? "e" : "-");
-            alloc_total += block->full_size + sizeof(HeapBlock);
+            alloc_total += current->full_size + sizeof(HeapBlock);
         }
         else
         {
-            kdbgln("- Used block (%p), of size %zu, of which %zu bytes are being used (%s%s)", (void*)block,
+            kdbgln("- Used block (%p), of size %zu, of which %zu bytes are being used (%s%s)", (void*)current,
-                   block->full_size, block->req_size, block->status & BLOCK_START_MEM ? "b" : "-",
+                   current->full_size, current->req_size, current->status & BLOCK_START_MEM ? "b" : "-",
-                   block->status & BLOCK_END_MEM ? "e" : "-");
+                   current->status & BLOCK_END_MEM ? "e" : "-");
-            alloc_total += block->full_size + sizeof(HeapBlock);
+            alloc_total += current->full_size + sizeof(HeapBlock);
-            alloc_used += block->req_size;
+            alloc_used += current->req_size;
         }
-        block = heap.next(block).value_or(nullptr);
+        block = heap.next(current);
     }
 
     kdbgln("-- Total memory allocated for heap: %zu bytes", alloc_total);

luna/include/luna/LinkedList.h

@@ -2,13 +2,6 @@
 #include <luna/Option.h>
 #include <luna/TypeTraits.h>
 
-template <typename T> inline Option<T*> nonnull_or_error(T* ptr)
-{
-    if (ptr == nullptr) return {};
-    else
-        return ptr;
-}
-
 template <typename T> class LinkedList;
 
 template <typename T> class LinkedListNode
@@ -131,32 +124,34 @@ template <typename T> class LinkedList
 
     Option<T*> first()
     {
-        return nonnull_or_error((T*)m_start_node);
+        return nonnull_or_empty_option((T*)m_start_node);
     }
 
     T* expect_first()
     {
-        return first().value();
+        check(m_start_node);
+        return (T*)m_start_node;
     }
 
     Option<T*> last()
     {
-        return nonnull_or_error((T*)m_end_node);
+        return nonnull_or_empty_option((T*)m_end_node);
     }
 
     T* expect_last()
     {
-        return last().value();
+        check(m_end_node);
+        return (T*)m_end_node;
     }
 
     Option<T*> next(T* item)
     {
-        return nonnull_or_error((T*)extract_node(item)->get_next());
+        return nonnull_or_empty_option((T*)extract_node(item)->get_next());
     }
 
     Option<T*> previous(T* item)
     {
-        return nonnull_or_error((T*)extract_node(item)->get_last());
+        return nonnull_or_empty_option((T*)extract_node(item)->get_last());
     }
 
     // Iterates over the elements of the LinkedList from start to end, calling callback for every element.

luna/include/luna/Option.h

@@ -30,6 +30,31 @@ template <typename T> class Option
         if (m_has_value) { m_storage.store_moved_reference(move(other.m_storage.fetch_reference())); }
     }
 
+    Option<T>& operator=(const Option<T>& other)
+    {
+        if (this == &other) return *this;
+
+        if (m_has_value) m_storage.destroy();
+        m_has_value = other.m_has_value;
+
+        if (m_has_value) { m_storage.store_reference(other.m_storage.fetch_reference()); }
+
+        return *this;
+    }
+
+    Option<T>& operator=(Option<T>&& other)
+    {
+        if (this == &other) return *this;
+
+        if (m_has_value) m_storage.destroy();
+        m_has_value = other.m_has_value;
+        other.m_has_value = false;
+
+        if (m_has_value) { m_storage.store_moved_reference(move(other.m_storage.fetch_reference())); }
+
+        return *this;
+    }
+
     Option() : m_has_value(false)
     {
     }
@@ -142,4 +167,11 @@ template <typename T> class Option
     };
     Storage m_storage;
     bool m_has_value { false };
-};
+};
+
+template <typename T> inline Option<T*> nonnull_or_empty_option(T* ptr)
+{
+    if (ptr == nullptr) return {};
+    else
+        return ptr;
+}

luna/include/luna/Result.h

@@ -202,3 +202,10 @@ template <> class Result<void>
         if (!_expr_rc.has_value()) return _expr_rc.release_error();                                                    \
         _expr_rc.release_value();                                                                                      \
     })
+
+template <typename T> inline Result<T*> nonnull_or_error(T* ptr, int error)
+{
+    if (ptr == nullptr) return err(error);
+    else
+        return ptr;
+}