From b574970965ea9a60e17f2d1ffd991dac0efff5e7 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 29 Oct 2025 09:03:17 +0000 Subject: [PATCH] Initial plan Refactor duplicated code in command handler and printf functions Co-authored-by: antoineverin <24255551+antoineverin@users.noreply.github.com> Refactor page table access duplication in paging.c Co-authored-by: antoineverin <24255551+antoineverin@users.noreply.github.com> Refactor PIC IRQ mask functions to eliminate duplication Co-authored-by: antoineverin <24255551+antoineverin@users.noreply.github.com> Final review - all refactoring complete Co-authored-by: antoineverin <24255551+antoineverin@users.noreply.github.com> Delete _codeql_detected_source_root Signed-off-by: Antoine Verin <24255551+antoineverin@users.noreply.github.com> apply ql sugestion --- kernel/include/command.h | 1 + kernel/src/command/halt.c | 6 ++++++ kernel/src/command/handle.c | 35 +++++++++++++++++++---------------- kernel/src/memory/paging.c | 33 +++++++++++++-------------------- kernel/src/pic.c | 25 ++++++++++--------------- kernel/src/tty/printk.c | 2 +- kernel/src/tty/vprintf.c | 8 ++------ 7 files changed, 52 insertions(+), 58 deletions(-) create mode 100644 kernel/src/command/halt.c diff --git a/kernel/include/command.h b/kernel/include/command.h index 58f5b04..aea00e1 100644 --- a/kernel/include/command.h +++ b/kernel/include/command.h @@ -4,5 +4,6 @@ void cmd_handle(const char* command); void cmd_ping(); void cmd_crash(); void cmd_reboot(); +void cmd_halt(); void cmd_clear(); void cmd_stack(); diff --git a/kernel/src/command/halt.c b/kernel/src/command/halt.c new file mode 100644 index 0000000..8121de0 --- /dev/null +++ b/kernel/src/command/halt.c @@ -0,0 +1,6 @@ +#include "command.h" +#include "io.h" + +void cmd_halt() { + outw(0x604, 0x2000); // QEMU specific +} diff --git a/kernel/src/command/handle.c b/kernel/src/command/handle.c index 9902343..8e2b33a 100644 --- a/kernel/src/command/handle.c +++ b/kernel/src/command/handle.c @@ -1,24 +1,27 @@ #include "command.h" -#include "io.h" #include "utils.h" #include #include +typedef struct { + const char* name; + void (*handler)(void); +} command_t; + +static const command_t commands[] = { + {"ping", &cmd_ping}, {"crash", &cmd_crash}, {"reboot", &cmd_reboot}, + {"halt", &cmd_halt}, {"clear", &cmd_clear}, {"stack", &cmd_stack}, + {NULL, NULL} // sentinel +}; + void cmd_handle(const char* command) { - if (strcmp(command, "ping") == 0) { - cmd_ping(); - } else if (strcmp(command, "crash") == 0) { - cmd_crash(); - } else if (strcmp(command, "reboot") == 0) { - cmd_reboot(); - } else if (strcmp(command, "halt") == 0) { - outw(0x604, 0x2000); // QEMU specific - } else if (strcmp(command, "clear") == 0) { - cmd_clear(); - } else if (strcmp(command, "stack") == 0) { - cmd_stack(); - } else { - if (strlen(command) > 0) - printf("%s: command not found\n", command); + for (size_t i = 0; commands[i].name != NULL; i++) { + if (strcmp(command, commands[i].name) == 0) { + commands[i].handler(); + return; + } } + + if (strlen(command) > 0) + printf("%s: command not found\n", command); } diff --git a/kernel/src/memory/paging.c b/kernel/src/memory/paging.c index 7a5e6f0..25bfa91 100644 --- a/kernel/src/memory/paging.c +++ b/kernel/src/memory/paging.c @@ -42,6 +42,14 @@ static uintptr_t get_page_table_phys(uintptr_t virt_addr, bool create) { return pt_phys; } +static inline pte_t* get_page_table(uint32_t pd_idx, uintptr_t pt_phys) { + if (recursive_active()) { + return pt_virt_for_index(pd_idx); + } else { + return (pte_t*)pt_phys; + } +} + void map_page(uintptr_t virt_addr, uintptr_t phys_addr, uint32_t flags) { virt_addr = page_align_down(virt_addr); phys_addr = page_align_down(phys_addr); @@ -50,12 +58,7 @@ void map_page(uintptr_t virt_addr, uintptr_t phys_addr, uint32_t flags) { uint32_t pd_idx = pd_index(virt_addr); uint32_t pt_idx = pt_index(virt_addr); - pte_t* pt; - if (recursive_active()) { - pt = pt_virt_for_index(pd_idx); - } else { - pt = (pte_t*)pt_phys; - } + pte_t* pt = get_page_table(pd_idx, pt_phys); pt[pt_idx] = phys_addr | flags; invlpg(virt_addr); @@ -70,13 +73,8 @@ void unmap_page(uintptr_t virt_addr) { if (!(page_directory[pd_idx] & PAGE_PRESENT)) return; - pte_t* pt; - if (recursive_active()) { - pt = pt_virt_for_index(pd_idx); - } else { - uintptr_t pt_phys = page_align_down(page_directory[pd_idx]); - pt = (pte_t*)pt_phys; - } + uintptr_t pt_phys = page_align_down(page_directory[pd_idx]); + pte_t* pt = get_page_table(pd_idx, pt_phys); pt[pt_idx] = 0; invlpg(virt_addr); @@ -104,13 +102,8 @@ uintptr_t get_phys_addr(uintptr_t virt_addr) { return 0; } - const pte_t* pt; - if (recursive_active()) { - pt = pt_virt_for_index(pd_idx); - } else { - uintptr_t pt_phys = page_align_down(page_directory[pd_idx]); - pt = (pte_t*)pt_phys; - } + uintptr_t pt_phys = page_align_down(page_directory[pd_idx]); + const pte_t* pt = get_page_table(pd_idx, pt_phys); if (!(pt[pt_idx] & PAGE_PRESENT)) { return 0; diff --git a/kernel/src/pic.c b/kernel/src/pic.c index 8177006..949a83d 100644 --- a/kernel/src/pic.c +++ b/kernel/src/pic.c @@ -1,5 +1,6 @@ #include "pic.h" #include "io.h" +#include void outb_wait(uint16_t port, uint8_t val) { outb(port, val); @@ -12,7 +13,7 @@ void pic_eoi(uint8_t irq) { outb(PIC_PARENT_COMMAND, PIC_EOI); } -void pic_enable_irq(uint8_t irq) { +static void pic_set_irq_mask(uint8_t irq, bool enable) { uint16_t port; uint8_t mask; @@ -23,25 +24,19 @@ void pic_enable_irq(uint8_t irq) { irq -= 8; } - mask = inb(port) & ~(1 << irq); - outb(port, mask); -} - -void pic_disable_irq(uint8_t irq) { - uint16_t port; - uint8_t mask; - - if (irq < 8) { - port = PIC_PARENT_DATA; + mask = inb(port); + if (enable) { + mask &= ~(1 << irq); } else { - port = PIC_CHILD_DATA; - irq -= 8; + mask |= (1 << irq); } - - mask = inb(port) | (1 << irq); outb(port, mask); } +void pic_enable_irq(uint8_t irq) { pic_set_irq_mask(irq, true); } + +void pic_disable_irq(uint8_t irq) { pic_set_irq_mask(irq, false); } + void pic_remap(int parent_offset, int child_offset) { outb_wait(PIC_PARENT_COMMAND, PIC_ICW1_INIT); outb_wait(PIC_CHILD_COMMAND, PIC_ICW1_INIT); diff --git a/kernel/src/tty/printk.c b/kernel/src/tty/printk.c index 959a6ff..473f7af 100644 --- a/kernel/src/tty/printk.c +++ b/kernel/src/tty/printk.c @@ -8,7 +8,7 @@ char* level_name[] = {"EMERG", "ALERT", "CRIT", "ERR", "WARN", "NOTICE", "INFO", "DEBUG", "", "CONT"}; static int get_level(char c) { - if (c >= '0' || c <= '7') + if (c >= '0' && c <= '7') return c - '0'; else if (c == 'c') return 9; diff --git a/kernel/src/tty/vprintf.c b/kernel/src/tty/vprintf.c index 07b0065..2dd38bb 100644 --- a/kernel/src/tty/vprintf.c +++ b/kernel/src/tty/vprintf.c @@ -20,17 +20,13 @@ int putnbr_base_unsigned(unsigned int n, const char* base) { int putnbr_base(long n, const char* base) { if (!base) return 0; - int len = 1; - size_t base_size = strlen(base); + int len = 0; if (n < 0) { tty_putchar('-'); n = -n; ++len; } - if (n / base_size) - len += putnbr_base(n / base_size, base); - tty_putchar(base[n % base_size]); - return len; + return len + putnbr_base_unsigned((unsigned int)n, base); } int putstr_count(char* str) {