Development #148
Development #148
Conversation
WalkthroughRefactors build to centralize source lists into cmake/source.cmake, replaces boolean heap option with string VF_CONFIG_HEAP_LANG and validates it, updates CMake messaging and flags, adds C/Rust heap backend switching via macros in mm/KernelHeap.h, removes rsalloc shell command and deletes scripts/mangle.py; updates README and build flags. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor DevEnv
participant CMake as CMake configure
participant Kernel as Runtime
participant HeapAPI as mm/KernelHeap.h
participant CHeap as KernelHeap C impl
participant RHeap as KernelHeap Rust impl
rect rgb(245,250,255)
note left of CMake: User sets -DVF_CONFIG_HEAP_LANG=<C/RUST>
DevEnv->>CMake: configure project
CMake->>CMake: validate VF_CONFIG_HEAP_LANG (RUST/C) or fatal
CMake->>Kernel: generate build with VF_CONFIG_HEAP_RUST or VF_CONFIG_HEAP_C
end
rect rgb(230,240,255)
note over HeapAPI,Kernel: Compile-time API mapping via macros or prototypes
Kernel->>HeapAPI: KernelMemoryAlloc(size)
alt VF_CONFIG_HEAP_LANG == "C"
HeapAPI->>CHeap: KernelMemoryAlloc(size)
CHeap-->>Kernel: ptr
else VF_CONFIG_HEAP_LANG == "RUST"
HeapAPI->>RHeap: rust_kmalloc(size)
RHeap-->>Kernel: ptr
end
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 inconclusive)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (4)
🧰 Additional context used🧬 Code graph analysis (1)mm/KernelHeap.h (1)
🔇 Additional comments (8)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
cmake/source.cmake (1)
136-141: Consider usinglist(APPEND)for consistency.While functionally correct, using
list(APPEND OBJ_SOURCES ...)instead ofset(OBJ_SOURCES ...)would be more consistent with the conditional source addition pattern at lines 131-133 and more maintainable if OBJ_SOURCES is extended elsewhere in the future.Apply this diff for consistency:
if(NOT EXCLUDE_EXTRA_OBJECTS) - set(OBJ_SOURCES + list(APPEND OBJ_SOURCES kernel/etc/objects/splash1.o kernel/etc/objects/panic.o ) endif()
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
CMakeLists.txt(2 hunks)README.md(2 hunks)cmake/cache.cmake(1 hunks)cmake/features.cmake(1 hunks)cmake/source.cmake(1 hunks)kernel/etc/Shell.c(0 hunks)mm/KernelHeap.h(2 hunks)scripts/mangle.py(0 hunks)
💤 Files with no reviewable changes (2)
- scripts/mangle.py
- kernel/etc/Shell.c
🔇 Additional comments (6)
CMakeLists.txt (2)
18-18: Centralized source configuration looks good.
Addinginclude(source)keeps the top-level CMake tidy while letting the new module manage source aggregation.
59-60: Status messaging aligns with new heap configuration.
ReportingVF_CONFIG_HEAP_LANG(and confirming sources configured) gives immediate feedback that the new language-selectable heap path is active.cmake/cache.cmake (1)
7-8: LGTM! String-based configuration is more explicit.The refactor from boolean to string-based heap language selection is well-structured and follows the same pattern as
VF_SCHEDULER. This approach is more explicit and extensible.Verify the default value change is intentional.
The default is now "RUST". If the previous
VF_CONFIG_HEAP_RUSTboolean defaulted toOFF(meaning C heap), this changes the default behavior. Please confirm this is intentional for existing users.cmake/source.cmake (3)
4-110: LGTM! Well-organized source grouping.The source groups are logically organized by functional area (core, scheduling, memory management, filesystems, drivers, etc.) and follow CMake conventions.
112-123: LGTM! Appropriate C source aggregation.C_SOURCES correctly aggregates C source groups while intentionally excluding ASM_SOURCES, CPP_SOURCES, and OBJ_SOURCES, which are typically handled separately in CMake build configurations.
130-134: Verify Rust heap backend configuration.The conditional only handles the "C" case for VF_CONFIG_HEAP_LANG. Ensure that when VF_CONFIG_HEAP_LANG equals "RUST", the Rust heap implementation sources are properly configured elsewhere in the build system.
Run the following script to verify Rust heap backend handling:
Additionally, consider adding a comment explaining where the Rust heap backend is configured for better maintainability.
1 participant
Summary by CodeRabbit
New Features
Refactor
Documentation
Chores