From afa77d29708b8e16f24a2bc9dea1043a5add438e Mon Sep 17 00:00:00 2001
From: pigeonmoelleux <pigeonmoelleux@crans.org>
Date: Thu, 18 May 2023 17:14:13 +0200
Subject: [PATCH] feat: logger can print one log at time

---
 Cargo.lock                       |  7 ++++++
 Cargo.toml                       |  1 +
 src/kernel/device/ata.rs         | 18 ++++++++------
 src/kernel/interrupt/keyboard.rs | 10 ++++++--
 src/syslog.rs                    | 40 +++++++++++++++++++++-----------
 5 files changed, 54 insertions(+), 22 deletions(-)

diff --git a/Cargo.lock b/Cargo.lock
index 59926ff..542301c 100644
--- a/Cargo.lock
+++ b/Cargo.lock
@@ -72,6 +72,12 @@ version = "1.0.0"
 source = "registry+https://github.com/rust-lang/crates.io-index"
 checksum = "baf1de4339761588bc0619e3cbc0120ee582ebb74b53b4efbf79117bd2da40fd"
 
+[[package]]
+name = "circular-buffer"
+version = "0.1.1"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "5887fa83bbf76a5cf15915add06c187cfc9d00f2ae9aa1f33d108a02ba7af345"
+
 [[package]]
 name = "conquer-once"
 version = "0.3.2"
@@ -312,6 +318,7 @@ dependencies = [
  "anyhow",
  "bitflags 2.3.1",
  "bootloader_api",
+ "circular-buffer",
  "conquer-once",
  "crossbeam-queue",
  "derive_more",
diff --git a/Cargo.toml b/Cargo.toml
index 7a4a99d..ddee706 100644
--- a/Cargo.toml
+++ b/Cargo.toml
@@ -15,6 +15,7 @@ acpi = "4"
 anyhow = { version = "1", default-features = false }
 bitflags = "2"
 bootloader_api = "0.11"
+circular-buffer = { version = "0.1", default-features = false }
 conquer-once = { version = "0.3", default-features = false }
 crossbeam-queue = { version = "0.3", default-features = false, features = [ "alloc" ] }
 derive_more = "0.99"
diff --git a/src/kernel/device/ata.rs b/src/kernel/device/ata.rs
index c471f51..9ed8312 100644
--- a/src/kernel/device/ata.rs
+++ b/src/kernel/device/ata.rs
@@ -1339,17 +1339,21 @@ impl IdeController {
         let secondary_slave = Drive::initialize(Arc::clone(&secondary_bus), DriveRole::Slave);
 
         info!(
-            "Found an ATA controller at {:?} containing :\n\t* Primary master: {:?}\n\t* Primary slave: {:?}\n\t* Secondary master: {:?}\n\t* Secondary slave: {:?}",
+            "Found an ATA controller at {:?} containing :\n    * Primary master: {:?}\n    * Primary slave: {:?}\n    * Secondary master: {:?}\n    * Secondary slave: {:?}",
             device.location, primary_master, primary_slave, secondary_master, secondary_slave
         );
 
         let to_ref = |drive| Arc::new(Mutex::new(drive));
 
-        Some(Self {
-            primary_master: primary_master.ok().map(to_ref),
-            primary_slave: primary_slave.ok().map(to_ref),
-            secondary_master: secondary_master.ok().map(to_ref),
-            secondary_slave: secondary_slave.ok().map(to_ref),
-        })
+        if primary_master.is_err() && primary_slave.is_err() && secondary_master.is_err() && secondary_slave.is_err() {
+            None
+        } else {
+            Some(Self {
+                primary_master: primary_master.ok().map(to_ref),
+                primary_slave: primary_slave.ok().map(to_ref),
+                secondary_master: secondary_master.ok().map(to_ref),
+                secondary_slave: secondary_slave.ok().map(to_ref),
+            })
+        }
     }
 }
diff --git a/src/kernel/interrupt/keyboard.rs b/src/kernel/interrupt/keyboard.rs
index cdecac1..9c57ac8 100644
--- a/src/kernel/interrupt/keyboard.rs
+++ b/src/kernel/interrupt/keyboard.rs
@@ -151,7 +151,8 @@ impl KeyQueue {
     ///
     /// Moreover, the following keys combinations are used :
     /// - `CTRL` + `ALT` + `DEL`: reboots the computer (not implemented yet)
-    /// - `ALT` + `L`: flushes the logger
+    /// - `ALT` + `P`: displays all logs and clears the logger
+    /// - `ALT` + `L`: displays the oldest log then removes it from the logger
     #[allow(clippy::wildcard_enum_match_arm)]
     pub fn handle_key(key: DecodedKey) {
         /// Tabulation char
@@ -171,11 +172,16 @@ impl KeyQueue {
             DecodedKey::RawKey(KeyCode::ArrowLeft) => Self::send_csi("1D"),
             DecodedKey::Unicode(TAB) if shift_pressed => Self::send_csi("2"),
             DecodedKey::Unicode(DEL) if alt_pressed && ctrl_pressed => warn!("TODO: implement reboot"),
-            DecodedKey::Unicode('l') if alt_pressed => {
+            DecodedKey::Unicode('p') if alt_pressed => {
                 let mut logger = SYS_LOGGER.lock();
                 logger.display_messages();
                 logger.clear();
             },
+            DecodedKey::Unicode('l') if alt_pressed => {
+                let mut logger = SYS_LOGGER.lock();
+                logger.display_next_message();
+                logger.remove_oldest();
+            },
             DecodedKey::Unicode(key) => Self::push_key(key),
             _ => {},
         }
diff --git a/src/syslog.rs b/src/syslog.rs
index 1930e33..86a3ad1 100644
--- a/src/syslog.rs
+++ b/src/syslog.rs
@@ -1,11 +1,11 @@
 //! Simple logger for `SkavOS`
 
-use alloc::collections::BTreeMap;
 use alloc::format;
 use alloc::string::String;
 use core::fmt::Display;
 use core::sync::atomic::{AtomicU64, Ordering};
 
+use circular_buffer::CircularBuffer;
 use log::{Level, Log, Metadata, Record};
 
 use crate::kernel::screen::color::{Color, CYAN, ORANGE, RED, WHITE, YELLOW};
@@ -85,19 +85,32 @@ impl Message {
 #[derive(Debug)]
 pub struct SysLog {
     /// Messages that have not been flushed yet
-    messages: BTreeMap<MessageId, Message>,
+    messages: CircularBuffer<100, Message>,
 }
 
 impl SysLog {
     /// Creates a new logger
     const fn new() -> Self {
         Self {
-            messages: BTreeMap::new(),
+            messages: CircularBuffer::new(),
         }
     }
 
+    /// Prints the next message that have not been displayed yet
+    pub fn display_next_message(&mut self) {
+        if self.messages.is_empty() {
+            println!("Empty logger");
+            return;
+        }
+
+        // SAFETY: `self.messages` is not empty
+        unsafe {
+            self.messages.front().unwrap_unchecked().print();
+        };
+    }
+
     /// Prints all messages contained in the logger
-    pub fn display_messages(&mut self) {
+    pub fn display_messages(&self) {
         if self.messages.is_empty() {
             println!("Empty logger");
             return;
@@ -105,11 +118,16 @@ impl SysLog {
 
         println!("Logs:");
 
-        for message in self.messages.values() {
+        for message in &self.messages {
             message.print();
         }
     }
 
+    /// Removes the oldest message saved in the logger
+    pub fn remove_oldest(&mut self) {
+        self.messages.pop_front();
+    }
+
     /// Removes all the messages saved in the logger
     pub fn clear(&mut self) {
         self.messages.clear();
@@ -124,12 +142,7 @@ impl Log for Locked<SysLog> {
     fn log(&self, record: &Record) {
         let content = format!("{}", record.args());
         let message = Message::new(record.level(), content);
-        let message_id = message.id;
-
-        assert!(
-            self.lock().messages.insert(message.id, message).is_none(),
-            "The message with ID {message_id} has already been logged"
-        );
+        self.lock().messages.push_back(message);
     }
 
     fn flush(&self) {}
@@ -182,14 +195,15 @@ mod test {
                 .args(format_args!("Hello world! {}", 42_usize))
                 .build(),
         );
+
         let previous_message_id = NEXT_MESSAGE_ID.load(Ordering::Relaxed) - 1;
-        assert_eq!(&syslog.lock().messages[&MessageId(previous_message_id)], &Message {
+        assert_eq!(syslog.lock().messages.back().unwrap(), &Message {
             id: MessageId(previous_message_id),
             level: Level::Info,
             content: "Hello world! 42".to_owned()
         });
         syslog.log(&Record::builder().level(Level::Warn).args(format_args!("1 + {} = 2", 1_usize)).build());
-        assert_eq!(&syslog.lock().messages[&MessageId(previous_message_id + 1)], &Message {
+        assert_eq!(syslog.lock().messages.back().unwrap(), &Message {
             id: MessageId(previous_message_id + 1),
             level: Level::Warn,
             content: "1 + 1 = 2".to_owned()
-- 
GitLab