-
Notifications
You must be signed in to change notification settings - Fork 1
Development #20
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Development #20
Conversation
WalkthroughThis update introduces screen scrolling to the Changes
Sequence Diagram(s)sequenceDiagram
participant UserApp
participant Kernel
participant Screen
UserApp->>Kernel: SYS_WRITE(buffer, size)
Kernel->>Kernel: Copy buffer to kernel-local buffer (FastMemcpy)
Kernel->>Kernel: Null-terminate kernel buffer
Kernel->>Screen: PrintKernel(kernel buffer)
Note right of Screen: If screen full, scroll up
Possibly related PRs
Poem
✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (1)
Kernel/Drivers/Interrupts.c (1)
73-73
: Fix misleading comment that contradicts the code state.The comment says "Re-enabled" but the
FastDisplayTicks(tick_count)
call is actually commented out (disabled). This inconsistency could confuse future developers about the intended state.- // FastDisplayTicks(tick_count); // Re-enabled + // FastDisplayTicks(tick_count); // Disabled to reduce interrupt overhead
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
Kernel/Core/Kernel.c
(1 hunks)Kernel/Core/Kernel.h
(0 hunks)Kernel/Core/Panic.c
(3 hunks)Kernel/Drivers/Interrupts.c
(1 hunks)Kernel/Memory/Memory.c
(1 hunks)Kernel/System/Syscall.c
(2 hunks)
💤 Files with no reviewable changes (1)
- Kernel/Core/Kernel.h
🧰 Additional context used
🧠 Learnings (1)
Kernel/Drivers/Interrupts.c (1)
Learnt from: assembler-0
PR: assembler-0/VoidFrame#2
File: Kernel/Syscall.c:44-44
Timestamp: 2025-07-08T00:36:50.290Z
Learning: In IdtSetGate function calls, the parameters are: (interrupt_vector, handler_address, selector, flags). The interrupt vector (like 0x80) is the first argument, and the selector (like 0x08 for kernel code segment) is the third argument.
🧬 Code Graph Analysis (2)
Kernel/Memory/Memory.c (2)
Kernel/Memory/MemOps.c (1)
FastMemset
(4-34)Kernel/Memory/MemOps.h (1)
FastMemset
(6-6)
Kernel/System/Syscall.c (4)
Kernel/Memory/MemOps.c (1)
FastMemcpy
(36-58)Kernel/Memory/MemOps.h (1)
FastMemcpy
(7-7)Kernel/Core/Kernel.c (1)
PrintKernel
(25-56)Kernel/Core/Kernel.h (1)
PrintKernel
(6-6)
🔇 Additional comments (6)
Kernel/Core/Kernel.c (1)
41-54
: Excellent implementation of screen scrolling functionality.The scrolling logic correctly handles the transition when the cursor reaches the bottom of the screen. The implementation moves all lines up by one position, clears the last line, and repositions the cursor appropriately. This ensures continuous output without truncation.
Kernel/Core/Panic.c (1)
10-10
: Good optimization: removing unnecessary busy-wait loops.Eliminating busy-wait loops in panic handlers improves efficiency and reduces unnecessary CPU consumption during critical error conditions. The panic handlers still function correctly by printing error messages and halting the system.
Also applies to: 22-22, 34-34
Kernel/Memory/Memory.c (1)
15-16
: Excellent optimization using FastMemset for bitmap initialization.Replacing the manual loop with
FastMemset
significantly improves performance by leveraging optimized CPU instructions (SSE2) for bulk memory operations when available. The functionality remains identical while gaining substantial performance benefits.Kernel/System/Syscall.c (3)
6-6
: Good addition of MemOps.h for FastMemcpy functionality.Including the MemOps header enables the use of optimized memory operations for safer and more efficient syscall buffer handling.
31-42
: Excellent security and performance improvements to SYS_WRITE handling.The refactored approach provides significant benefits:
- Security: Limits buffer size and copies to kernel-controlled buffer
- Safety: Ensures null termination to prevent buffer overruns
- Performance: Uses optimized FastMemcpy and bulk printing via PrintKernel
- Efficiency: Eliminates character-by-character processing
32-32
: No changes needed:MAX_SYSCALL_BUFFER_SIZE
is defined
The constant is defined inKernel/System/Syscall.h:14
, so the usage inSyscall.c
is valid.
Summary by CodeRabbit
New Features
Performance Improvements
Bug Fixes
Other Changes