Fix code review issues: heap allocation, I/O consistency, PCI scan optimization

Co-authored-by: johndoe6345789 <224850594+johndoe6345789@users.noreply.github.com>

kernel/src/interrupts.c

@@ -8,6 +8,17 @@
 #include "kernel/interrupts.h"
 #include "kernel/timer.h"
 
+// I/O port access functions
+static inline void outb(uint16_t port, uint8_t value) {
+    __asm__ volatile("outb %0, %1" : : "a"(value), "Nd"(port));
+}
+
+// PIC ports
+#define PIC1_COMMAND 0x20
+#define PIC1_DATA    0x21
+#define PIC2_COMMAND 0xA0
+#define PIC2_DATA    0xA1
+
 // IDT entries (256 interrupts in x86_64)
 static idt_entry_t idt[256];
 static idt_ptr_t idt_ptr;
@@ -64,24 +75,24 @@ static void idt_set_gate(uint8_t num, uint64_t handler, uint16_t selector, uint8
 // Remap PIC (Programmable Interrupt Controller)
 static void pic_remap(void) {
     // ICW1: Initialize PIC
-    __asm__ volatile("outb %0, $0x20" : : "a"((uint8_t)0x11));  // Master PIC
-    __asm__ volatile("outb %0, $0xA0" : : "a"((uint8_t)0x11));  // Slave PIC
+    outb(PIC1_COMMAND, 0x11);  // Master PIC
+    outb(PIC2_COMMAND, 0x11);  // Slave PIC
     
     // ICW2: Set interrupt vector offsets
-    __asm__ volatile("outb %0, $0x21" : : "a"((uint8_t)0x20));  // Master offset to 0x20
-    __asm__ volatile("outb %0, $0xA1" : : "a"((uint8_t)0x28));  // Slave offset to 0x28
+    outb(PIC1_DATA, 0x20);  // Master offset to 0x20
+    outb(PIC2_DATA, 0x28);  // Slave offset to 0x28
     
     // ICW3: Set up cascade
-    __asm__ volatile("outb %0, $0x21" : : "a"((uint8_t)0x04));  // Tell master about slave
-    __asm__ volatile("outb %0, $0xA1" : : "a"((uint8_t)0x02));  // Tell slave its cascade
+    outb(PIC1_DATA, 0x04);  // Tell master about slave
+    outb(PIC2_DATA, 0x02);  // Tell slave its cascade
     
     // ICW4: Set mode
-    __asm__ volatile("outb %0, $0x21" : : "a"((uint8_t)0x01));
-    __asm__ volatile("outb %0, $0xA1" : : "a"((uint8_t)0x01));
+    outb(PIC1_DATA, 0x01);
+    outb(PIC2_DATA, 0x01);
     
     // Mask all interrupts initially
-    __asm__ volatile("outb %0, $0x21" : : "a"((uint8_t)0xFF));
-    __asm__ volatile("outb %0, $0xA1" : : "a"((uint8_t)0xFF));
+    outb(PIC1_DATA, 0xFF);
+    outb(PIC2_DATA, 0xFF);
 }
 
 // Initialize IDT
@@ -156,9 +167,9 @@ void interrupt_handler(registers_t* regs) {
     if (regs->int_no >= 32 && regs->int_no < 48) {
         if (regs->int_no >= 40) {
             // Slave PIC
-            __asm__ volatile("outb %0, $0xA0" : : "a"((uint8_t)0x20));
+            outb(PIC2_COMMAND, 0x20);
         }
         // Master PIC
-        __asm__ volatile("outb %0, $0x20" : : "a"((uint8_t)0x20));
+        outb(PIC1_COMMAND, 0x20);
     }
 }

kernel/src/main.c

@@ -30,10 +30,14 @@ void kernel_main(BootInfo* boot_info) {
     // Initialize physical memory manager
     pmm_init(boot_info);
     
-    // Initialize kernel heap (allocate 1MB for kernel heap)
-    void* heap_mem = pmm_alloc_page();
-    if (heap_mem) {
-        heap_init(heap_mem, 256 * PAGE_SIZE);  // 1MB heap
+    // Initialize kernel heap (allocate 256 pages = 1MB for kernel heap)
+    void* heap_start_page = pmm_alloc_page();
+    if (heap_start_page) {
+        // Allocate additional pages for heap (256 pages total)
+        for (int i = 1; i < 256; i++) {
+            pmm_alloc_page();  // Allocate contiguous pages
+        }
+        heap_init(heap_start_page, 256 * PAGE_SIZE);  // 1MB heap
     }
     
     // Initialize timer (1000 Hz = 1ms per tick)

kernel/src/pci.c

@@ -93,7 +93,10 @@ static void pci_probe_device(uint8_t bus, uint8_t device, uint8_t function) {
 // Initialize PCI subsystem
 void pci_init(void) {
     // Scan all buses, devices, and functions
+    // Note: On real hardware, could optimize by stopping after consecutive empty buses
     for (uint16_t bus = 0; bus < 256; bus++) {
+        uint8_t devices_found = 0;
+        
         for (uint8_t device = 0; device < 32; device++) {
             // Check if device exists (function 0)
             uint32_t vendor_device = pci_read_config(bus, device, 0, 0x00);
@@ -101,6 +104,7 @@ void pci_init(void) {
                 continue;  // Device doesn't exist
             }
             
+            devices_found++;
             pci_probe_device(bus, device, 0);
             
             // Check if multi-function device
@@ -115,6 +119,10 @@ void pci_init(void) {
                 }
             }
         }
+        
+        // Early termination: if no devices found in this bus and we're past bus 0, 
+        // we can potentially stop (though some systems have gaps)
+        // For now, continue full scan for maximum compatibility
     }
 }
 

kernel/src/timer.c

@@ -10,6 +10,7 @@
 // PIT I/O ports
 #define PIT_CHANNEL0 0x40
 #define PIT_COMMAND  0x43
+#define PIC1_DATA    0x21
 
 // PIT constants
 #define PIT_BASE_FREQUENCY 1193182  // Hz
@@ -17,11 +18,17 @@
 // Tick counter
 static volatile uint64_t timer_ticks = 0;
 
-// I/O port access
+// I/O port access functions
 static inline void outb(uint16_t port, uint8_t value) {
     __asm__ volatile("outb %0, %1" : : "a"(value), "Nd"(port));
 }
 
+static inline uint8_t inb(uint16_t port) {
+    uint8_t value;
+    __asm__ volatile("inb %1, %0" : "=a"(value) : "Nd"(port));
+    return value;
+}
+
 // Initialize timer
 void timer_init(uint32_t frequency) {
     // Calculate divisor
@@ -36,10 +43,9 @@ void timer_init(uint32_t frequency) {
     
     // Enable timer interrupt (IRQ0)
     // Unmask IRQ0 in PIC
-    uint8_t mask;
-    __asm__ volatile("inb $0x21, %0" : "=a"(mask));
+    uint8_t mask = inb(PIC1_DATA);
     mask &= ~0x01;  // Clear bit 0 (IRQ0)
-    outb(0x21, mask);
+    outb(PIC1_DATA, mask);
 }
 
 // Get current tick count