Development #5
Development #5
Conversation
WalkthroughThe changes introduce a boundary check in the kernel's printing function to prevent line overflow, update the syscall interrupt vector and segment selector usage, and add new constants for syscall configuration, including a buffer size limit. No function signatures or exported interfaces were altered. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Kernel
participant Syscall
participant IDT
User->>Kernel: Trigger syscall (int 0x80)
Kernel->>IDT: Use SYSCALL_SEGMENT_SELECTOR (0x08) for gate setup
Kernel->>Syscall: Handle syscall with buffer size limit (4096 bytes)
Syscall-->>User: Return result
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 (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
Kernel/Kernel.c(1 hunks)Kernel/Syscall.c(1 hunks)Kernel/Syscall.h(1 hunks)
🧰 Additional context used
🧠 Learnings (3)
📓 Common learnings
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.
Kernel/Syscall.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.
Kernel/Syscall.h (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 (1)
Kernel/Syscall.c (1)
Kernel/Idt.h (1)
IdtSetGate(25-25)
🔇 Additional comments (3)
Kernel/Syscall.c (1)
47-47: LGTM! Correct parameter usage in IdtSetGate.The change from
SYSCALL_INTERRUPT_VECTORtoSYSCALL_SEGMENT_SELECTORproperly uses the segment selector (0x08) as the third parameter, while maintaining the interrupt vector (0x80) as the first parameter. This aligns with the correctIdtSetGateparameter order.Kernel/Syscall.h (2)
11-11: LGTM! Standard syscall interrupt vector.Changing from 0x08 to 0x80 aligns with the standard Linux syscall interrupt vector, which is the conventional choice for system calls in x86 systems.
13-14: LGTM! Well-defined syscall constants.The addition of
SYSCALL_SEGMENT_SELECTOR(0x08) andMAX_SYSCALL_BUFFER_SIZE(4096) provides:
- Clear separation between interrupt vector and segment selector concepts
- Reasonable buffer size limit for syscall operations
- Better code maintainability through named constants
| } | ||
|
|
||
| void PrintKernelAt(const char *str, int line, int col) { | ||
| if (CurrentLine >= 25) CurrentLine = 24; |
There was a problem hiding this comment.
Fix logical inconsistency in boundary check.
The boundary check modifies the global CurrentLine variable, but the PrintKernelAt function uses the line parameter for positioning (line 104). This creates an inconsistency where the function doesn't use CurrentLine for its core functionality but still modifies it.
Consider either:
- Removing the boundary check if it's not needed for this function's operation
- Applying the boundary check to the
lineparameter instead - Clarifying the intended behavior of this global state modification
- if (CurrentLine >= 25) CurrentLine = 24;
+ // Remove if CurrentLine isn't used for positioning in this function
+ // OR apply boundary check to the line parameter:
+ // if (line >= 25) line = 24;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if (CurrentLine >= 25) CurrentLine = 24; | |
| // Remove if CurrentLine isn't used for positioning in this function | |
| // OR apply boundary check to the line parameter: | |
| // if (line >= 25) line = 24; |
🤖 Prompt for AI Agents
In Kernel/Kernel.c at line 102, the boundary check currently modifies the global
variable CurrentLine, but the function PrintKernelAt uses the local parameter
line for positioning, causing inconsistency. To fix this, apply the boundary
check to the line parameter instead of CurrentLine, ensuring the function's
logic aligns with its parameter usage and avoids unintended side effects on the
global state.
1 participant
Summary by CodeRabbit
Bug Fixes
New Features
Refactor