Removed messy debug logging, added print_oneshot!()

kernel/src/arch/x86_64/cpu/apic.rs

@@ -2,10 +2,13 @@
 
 use core::arch::x86_64::__cpuid;
 
-use crate::arch::x86_64::cpu::msr::*;
-use crate::arch::x86_64::memory::mapping::PHYSICAL_MEMORY_OFFSET;
-use crate::arch::x86_64::memory::{FRAME_ALLOCATOR, OFFSET_PAGE_TABLE};
-use crate::{debugln, serial_print, serial_println};
+use crate::arch::x86_64::{
+    cpu::msr::*,
+    memory::{
+        FRAME_ALLOCATOR, OFFSET_PAGE_TABLE, mapping::PHYSICAL_MEMORY_OFFSET,
+    },
+};
+
 use x86_64::structures::paging::Translate;
 use x86_64::{
     PhysAddr, VirtAddr,
@@ -28,7 +31,6 @@ const CPUID_FEAT_EDX_APIC: u64 = 1 << 9; // the cpuid instruction will return th
 
 fn set_apic_base_enable(apic: PhysAddr) {
     let rax = (apic.as_u64() & 0xfffff0000) | IA32_APIC_BASE_MSR_ENABLE;
-    debugln!("apic {:?}", PhysAddr::new(rax));
     cpu_set_msr(IA32_APIC_BASE_MSR, rax);
 }
 
@@ -42,8 +44,6 @@ fn get_apic_base() -> PhysAddr {
     let mut value: u64 = 0;
     cpu_get_msr(IA32_APIC_BASE_MSR, &mut value);
 
-    debugln!("apic base {:#x}", value);
-
     PhysAddr::new(value & 0xfffff0000)
 }
 
@@ -55,7 +55,6 @@ fn write_apic_register(apic_base: &PhysAddr, reg: u64, value: u32) {
 
     let phys_check =
         OFFSET_PAGE_TABLE.get().unwrap().lock().translate(virt_addr);
-    debugln!("{:?}", phys_check);
 
     unsafe { *(virt_addr.as_u64() as *mut u32) = value };
 }
@@ -76,14 +75,13 @@ pub fn check_apic() -> bool {
 unsafe fn phys_to_virt(phys: PhysAddr) -> VirtAddr {
     let phys = phys.as_u64();
     phys.checked_add(*PHYSICAL_MEMORY_OFFSET)
-        .map_or_else(|| panic!(" overflow"), VirtAddr::new)
+        .map_or_else(|| panic!("Overflow"), VirtAddr::new)
 }
 
 pub fn enable_apic() {
     let apic_base_physical_addr = get_apic_base();
     set_apic_base_enable(apic_base_physical_addr);
 
-    debugln!("{:?}", apic_base_physical_addr);
     enable_timer();
 
     write_apic_register(

kernel/src/arch/x86_64/drivers/ascii/mod.rs

@@ -1,4 +1,4 @@
-use core::fmt;
+use core::fmt::{self, Write};
 use spin::{Lazy, Mutex};
 use x86_64::instructions::interrupts;
 
@@ -7,6 +7,7 @@ use crate::arch::x86_64::drivers::framebuffer::{
 };
 
 use crate::resources::font::{FONT_SPLEEN_8X16, Font};
+use crate::std::maths::geometry::Vec2;
 
 static FONT_WIDTH: u32 = 8;
 static FONT_HEIGHT: u32 = 16;
@@ -25,11 +26,8 @@ pub struct Writer {
     screen_width: u32,
     /// Measured in chars not pixels.
     screen_height: u32,
-    /// 16 pixels tall.
     text_line: u32,
-    /// 8 pixels wide.
     text_col: u32,
-
     fg_color: Colour,
     bg_color: Colour,
 }
@@ -62,6 +60,15 @@ impl Writer {
         self.font = font;
     }
 
+    pub const fn set_pos(&mut self, coords: Vec2<u32>) {
+        self.text_col = coords.x();
+        self.text_line = coords.y();
+    }
+
+    pub const fn pos(&self) -> Vec2<u32> {
+        Vec2::new(self.text_col, self.text_line)
+    }
+
     /// This is sent when the user types a backspace.
     const BACKSPACE: u8 = 8;
 
@@ -206,6 +213,34 @@ pub fn clear_screen() {
     });
 }
 
+/// Prints a string at a given `coords` before returning to original position.
+///
+/// These are 0 based indices and refer to character position, not pixel
+/// positions.
+pub fn _print_oneshot(
+    fg: Colour,
+    bg: Colour,
+    coords: Vec2<u32>,
+    args: fmt::Arguments,
+) {
+    interrupts::without_interrupts(|| {
+        let mut writer = WRITER.lock();
+
+        // Save the old positions of `text_col` and `text_line`.
+        let old_pos = writer.pos();
+
+        // Also save the colors.
+        let old_fg_color = writer.fg_color;
+        let old_bg_color = writer.bg_color;
+
+        writer.set_pos(coords);
+        writer.set_colour(fg, bg);
+        writer.write_fmt(args).unwrap();
+        writer.set_colour(old_fg_color, old_bg_color);
+        writer.set_pos(old_pos);
+    });
+}
+
 pub fn reset_cursor() {
     interrupts::without_interrupts(|| {
         let mut writer = WRITER.lock();
@@ -214,6 +249,15 @@ pub fn reset_cursor() {
     });
 }
 
+#[macro_export]
+/// Prints a coloured string at a position before restoring previous position
+/// and colour.
+macro_rules! print_oneshot {
+    ($position:expr, $fg:expr, $bg:expr, $($arg:tt)*) => {
+        $crate::arch::x86_64::drivers::ascii::_print_oneshot($fg, $bg, $position, format_args!($($arg)*));
+    };
+}
+
 #[macro_export]
 macro_rules! println_log {
 	() => ($crate::print_log!("\n"));

kernel/src/arch/x86_64/memory/allocation/heap_alloc.rs

@@ -3,8 +3,7 @@ use crate::arch::x86_64::memory::units::MemoryUnits;
 use crate::arch::x86_64::memory::{
     FRAME_ALLOCATOR, HEAP_SIZE, HEAP_VIRTUAL_SPACE,
 };
-use crate::serial_println;
-use crate::{debugln, serial_print};
+use crate::debugln;
 use core::alloc::{GlobalAlloc, Layout};
 use core::ptr;
 use spin::{Mutex, MutexGuard};

kernel/src/lib.rs

@@ -1,5 +1,6 @@
 #![no_std]
 #![feature(abi_x86_interrupt)]
+#![feature(impl_trait_in_bindings)]
 #![warn(
     clippy::correctness,
     clippy::nursery,
@@ -14,8 +15,6 @@
 extern crate alloc;
 
 use crate::{
-    // TODO: Fix nesting under `arch`. A lot of code does not NEED to run on
-    // x86_64. Note that the panic handler does.
     arch::x86_64::{
         cpu::apic::enable_apic,
         drivers::{
@@ -26,34 +25,29 @@ use crate::{
             allocation::{
                 heap_alloc::init_heap, page_alloc::FoundryOSFrameAllocator,
             },
-            init_page_table,
+            init_page_table, mapping,
             units::MemoryUnits,
         },
     },
     prelude::*,
 };
 
-use arch::x86_64::memory::mapping;
+use alloc::{boxed::Box, format};
 use core::arch::asm;
 use limine::BaseRevision;
 use std::unwind::UNWINDER;
 use x86_64::VirtAddr;
 
 pub mod arch;
+/// Commonly used re-exports.
+pub mod prelude;
 pub mod resources;
-#[allow(unused)] // We aren't using much of this right now.
+// We aren't using much of this right now.
+#[allow(unused)]
 pub mod std;
+mod step;
 pub mod util;
 
-pub mod prelude {
-    pub use crate::{
-        debug, debugln, eprint, eprintln, print, print_log, println,
-        println_log, serial_print, serial_println,
-        std::debug::_debug,
-        std::io::{_print, _print_err, _print_log, _serial_write},
-    };
-}
-
 /// Sets the base revision to the latest revision supported by the crate.
 /// See specification for further info.
 /// Be sure to mark all limine requests with #[used], otherwise they may be
@@ -73,12 +67,25 @@ pub fn hcf() -> ! {
     }
 }
 
+#[derive(Debug)]
+pub struct NoError {}
+
+impl core::fmt::Display for NoError {
+    fn fmt(&self, _f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result {
+        Ok(())
+    }
+}
+
+pub struct NoTags;
+
+impl core::error::Error for NoError {}
+
 /// Panicking before this is initialised is unwise. We should probably extract
 /// very early init into it's own function because Stack Traces may require
 /// allocations etc.
-pub fn boot() -> Result<(), &'static str> {
+pub fn boot() -> Result<(), Box<dyn core::error::Error>> {
     if !BASE_REVISION.is_supported() {
-        return Err("Base revision not supported");
+        return Err("Base revision not supported.".into());
     }
 
     use arch::x86_64::{gdt, interrupts};
@@ -116,22 +123,22 @@ pub fn boot() -> Result<(), &'static str> {
     FoundryOSFrameAllocator::init(memory_map);
     let available_bytes =
         FRAME_ALLOCATOR.get().unwrap().lock().available_memory();
+
     debugln!(
         "      => Available Memory: {}",
         MemoryUnits::from_bytes(available_bytes as usize)
     );
 
-    debugln!("[Success]");
-
+    // Allocations should be all fine past this point.
     debugln!("    Initialising Heap... ");
-    if unsafe { init_heap() }.is_err() {
-        return Err("Failed to initialise heap: error");
+    match unsafe { init_heap() } {
+        Ok(_) => debugln!("    [Success]"),
+        Err(why) => return Err(format!("{:?}", why).into()),
     }
-    debugln!("    [Success]");
 
-    // debug!("    Enabling PICs... ");
-    // interrupts::enable_pic();
-    // debugln!("[Success]");
+    debug!("    Enabling PICs... ");
+    interrupts::enable_pic();
+    debugln!("[Success]");
 
     debug!("    Disabling PICs... ");
     interrupts::disable_pic();
@@ -145,7 +152,9 @@ pub fn boot() -> Result<(), &'static str> {
     x86_64::instructions::interrupts::enable();
     debugln!("[Success]");
 
-    UNWINDER.lock(); // Initialises the Unwinder once and only once :fingers_crossed:.
+    // Initialises the Unwinder once and only once because this makes a heap
+    // allocation.
+    UNWINDER.lock();
 
     Ok(())
 }

kernel/src/prelude.rs

@@ -0,0 +1,7 @@
+pub use crate::{
+    arch::x86_64::drivers::framebuffer::colour::Colour,
+    debug, debugln, eprint, eprintln, print, print_log, print_oneshot, println,
+    println_log, serial_print, serial_println,
+    std::debug::_debug,
+    std::io::{_print, _print_err, _print_log, _serial_write},
+};

kernel/src/std/elf/mod.rs

@@ -84,15 +84,13 @@ impl ElfReader {
         Ok(Self { elf })
     }
 
-    #[warn(clippy::unwrap_used)]
     pub fn get_symbol_table(
         &self,
     ) -> Result<
         Option<(SymbolTable<'static, LittleEndian>, StringTable<'static>)>,
         ElfError,
     > {
-        // TODO: Remove .unwrap().
-        Ok(self.elf.symbol_table().unwrap())
+        Ok(self.elf.symbol_table()?)
     }
 
     /// Gets the section size of `section_name` in bytes.

kernel/src/std/maths/geometry.rs

@@ -42,6 +42,14 @@ impl<T: Coordinate> Vec2<T> {
     pub const fn y(&self) -> T {
         self.y
     }
+
+    pub const fn set_x(&mut self, x: T) {
+        self.x = x;
+    }
+
+    pub const fn set_y(&mut self, y: T) {
+        self.y = y;
+    }
 }
 
 impl<T: Coordinate> AddAssign for Vec2<T> {

kernel/src/std/unwind/panic.rs

@@ -16,8 +16,7 @@ use crate::{
 #[panic_handler]
 /// A basic panic handler which can produce a helpful stack trace.
 pub fn panic_handler(info: &PanicInfo<'_>) -> ! {
-    println!("Kernel panic: {}", info);
-    serial_println!("Kernel panic: {}", info);
+    debugln!("Kernel panic: {}", info);
 
     let mut rip: u64;
     let mut rsp: u64;
@@ -33,18 +32,18 @@ pub fn panic_handler(info: &PanicInfo<'_>) -> ! {
 
     while let Some(call_frame) = unwinder.next().unwrap_or_else(|err| {
         // If an unwind error occurred.
-        eprintln!("{:?}", err);
+        debugln!("{:?}", err);
         hcf()
     }) {
         serial_println!("Got frame: {:x?}", call_frame);
         let Some((symtab, strtab)) =
             ELF.get_symbol_table().unwrap_or_else(|e| {
-                serial_println!("{:?}", e);
+                debugln!("{:?}", e);
                 hcf()
             })
         else {
             // TODO: Omit symbol names but just print addresses.
-            serial_println!("Didn't find symtab and strtab!");
+            debugln!("Didn't find symtab and strtab!");
             hcf()
         };
 

kernel/src/std/unwind/unwinder.rs

@@ -126,7 +126,6 @@ impl FallibleIterator for Unwinder {
 
         Ok(Some(CallFrame { pc, symbol: 0 }))
     }
-    // fn next(&mut self) -> Option<Result<Option<CallFrame>, UnwinderError>> {}
 }
 
 impl Unwinder {

kernel/src/step.rs

@@ -0,0 +1,104 @@
+/* //! Defines the [Step] struct which may be used when initialising the system.
+//!
+//! # Warning
+//!
+//! Use of alloc is currently required, so early initialisation will need a
+//! different method. We should possibly abstract some functionality so that
+//! consumers don't need to worry or care about the difference.
+use alloc::{borrow::ToOwned, boxed::Box, string::String, vec::Vec};
+
+use crate::{arch::x86_64::drivers::ascii::WRITER, prelude::*};
+
+/// Represents a [Step] when initialising the system.
+///
+/// Handles printing various information to the framebuffer using steps.
+///
+/// # Example
+///
+/// ```rs
+/// // Setup a Step for Foo.
+/// let foo_step: Step<Foo, FooError> = Step::new("foo", Foo::init()).register_tag(|foo| { foo.bar().to_string() });
+/// // This returns a Result<T, E>
+/// let foo = foo_step.run()?;
+/// ```
+///
+/// # TODOs
+///
+/// * If T = Option<T> and None is returned, we print `"[Not found]"`, otherwise
+///   `"[Success]`".
+pub struct Step<T: Clone + ToOwned> {
+    /// The initialisation function for the Step, usually this is a closure.
+    init_fn: fn() -> T,
+    /// A list of tag generator functions to display under the Step on success.
+    /// They are displayed like:
+    /// ```
+    ///     Initialising module... [Success]
+    ///         => Some information here.
+    /// ```
+    ///
+    /// # TODOs
+    ///
+    /// * Support tags without alloc being initialised.
+    tags: Vec<Box<dyn Fn(T) -> String>>,
+    /// The name of the module being initialised.
+    task: &'static str,
+}
+
+// This impl block might be so horrible it needs its own file.
+impl<T: ToOwned + Clone> Step<T> {
+    pub fn new(
+        task: &'static str,
+        init_fn: fn() -> T,
+        tags: Vec<Box<dyn Fn(T) -> String>>,
+    ) -> Self {
+        Self {
+            init_fn,
+            tags,
+            task,
+        }
+    }
+
+    /// Runs the initialisation function and logs where required.
+    ///
+    /// This function is a little messy but it just handles pretty printing the
+    /// [Step] to the terminal for us.
+    pub fn run(self) -> T {
+        print_log!("    {}", self.task);
+        let mut success_position = WRITER.lock().pos();
+        success_position.set_x(success_position.x() + 1);
+
+        let ret = (self.init_fn)();
+
+        println!();
+
+        for tag in &self.tags[..] {
+            // Calls the closure on the return value.
+            println_log!("      => {}", (tag)(ret.clone()));
+        }
+
+        // Print whether or not the step succeeded.
+        print_oneshot!(
+            success_position,
+            Colour::Green,
+            Colour::Black,
+            "[Success]"
+        );
+
+        ret
+    }
+
+    /// Registers a 'tag' or note to display under the line which says
+    /// `Initialising module... [Success]`.
+    ///
+    /// # Notes
+    ///
+    /// The tag is a closure taking in a reference to T and returning a
+    /// presumably formatted String.
+    #[allow(unused)]
+    pub fn register_tag(&mut self, tag: Box<dyn Fn(T) -> String>) -> &Self {
+        self.tags.push(tag);
+
+        self
+    }
+}
+ */

rustfmt.toml

@@ -2,3 +2,4 @@ wrap_comments = true
 max_width = 80
 comment_width = 80
 format_code_in_doc_comments = true
+doc_comment_code_block_width = 80