Teeny Tiny patch#1
Conversation
ReaQwQ
left a comment
There was a problem hiding this comment.
Thanks for the patch! This is a helpful developer-experience cleanup, especially adding rust-toolchain.toml and making run.sh handle target setup more explicitly.
I found one lint issue that needs to be fixed before merging, plus two small cleanup/consistency notes. Nothing here looks conceptually wrong; the main issue is that Rust recently changed the CPUID intrinsic safety contract for x86_64/i686-style targets, so the added unsafe block now trips this repo's warning-deny lint.
Validation I ran locally:
- cargo fmt --check: passed
- cargo check --target x86_64-unknown-uefi: passed, with an unused_unsafe warning
- cargo clippy --all-targets --all-features -- -D warnings: failed on the unused unsafe block
- just lint: failed at the same unused unsafe block
- just architecture-boundaries: passed
- userland clippy for x86_64-unknown-none: passed
- bash -n run.sh: passed
One small consistency request: could we also mirror the new target setup in run.bat so Windows users get the same setup path as run.sh?
The top-level kernel build runs build.rs, which builds userland demos for x86_64-unknown-none, so Windows users can still hit the manual “install this target” failure path even though run.sh now handles it. Keeping x86_64-unknown-none in rust-toolchain.toml looks right; this is mainly about keeping run.sh and run.bat consistent.
| pub fn has_apic() -> bool { | ||
| let cpuid = core::arch::x86_64::__cpuid(1); | ||
| // SAFETY, CPUID leaf 1 is available on all AMD64 CPUs. | ||
| let cpuid = unsafe { core::arch::x86_64::__cpuid(1) }; |
There was a problem hiding this comment.
Could you remove this unsafe block?
I see why it was added, since __cpuid used to be an unsafe intrinsic. Recent stdarch changed that for x86_64/i686-style targets, though, so with the toolchain selected by this PR, core::arch::x86_64::__cpuid(1) is safe and this currently fails clippy as unused_unsafe.
Removing the unsafe { ... } wrapper and the safety comment should fix the lint failure.
Reference: rust-lang/stdarch#1935
|
|
||
| # Files | ||
| GEMINI.md | ||
| nul |
There was a problem hiding this comment.
Tiny cleanup: this line seems to include a hidden private-use character after nul (EF 80 8D in UTF-8). Could you replace it with plain nul, or remove the line if it was accidental?
While touching this file, adding a final newline would also keep the diff cleaner.
| @@ -0,0 +1,7 @@ | |||
| [toolchain] | |||
| channel = "nightly" | |||
There was a problem hiding this comment.
Question/nit: since channel = "nightly" isn't date-pinned, this toolchain could
silently shift over time — today's unused_unsafe situation with __cpuid is
actually a good example of how a nightly-only safety-contract change can flip a
build from passing to failing between two people's local runs (or between CI runs
on different days).
Would it make sense to pin this to a dated nightly (e.g. nightly-2026-06-15) for
reproducibility? Not blocking this PR, just flagging for discussion.
Remove 'nul' and add 'null' to .gitignore
No description provided.