Skip to content

Refactor sleep.rs#3

Open
Big-Iron-Cheems wants to merge 8 commits intovalignatev:masterfrom
Big-Iron-Cheems:feat/rust-patch
Open

Refactor sleep.rs#3
Big-Iron-Cheems wants to merge 8 commits intovalignatev:masterfrom
Big-Iron-Cheems:feat/rust-patch

Conversation

@Big-Iron-Cheems
Copy link

This pull request refactors and improves sleep.rs, focusing on safety, idiomatic Rust, and robust error handling.
Each commit message should explain why I did these changes, with annexed sources for more techinical ones.

Key improvements include:

  • Explicit compile time warning for unsupported targets.
  • No busy waiting in loops via hlt instruction.
  • More idiomatic error reporting with a SleepError enum and report_error function.
  • Safer and simpler handling of raw pointers and argv strings.

Note: I believe that the Rust implementation could benefit from providing Cargo.toml and .cargo/config.toml.
Personally I settled with the following versions:

[package]
name = "sleep"
edition = "2024"

[[bin]]
name = "sleep"
path = "sleep.rs"
test = false
bench = false

[profile.dev]
panic = "abort"
opt-level = 0
lto = false
codegen-units = 1

[profile.release]
panic = "abort"
opt-level = 3
lto = true
codegen-units = 1

[dependencies]
[build]
target = "x86_64-unknown-linux-gnu"

[target.x86_64-unknown-linux-gnu]
rustflags = [
    "-C", "link-args=-nostartfiles",
    "-C", "panic=abort",
    "-C", "opt-level=3",
    "-C", "strip=debuginfo"
]

It would be possible to have a structure like this, but it is out of scope.

.
├── rust/
│   ├── .cargo/
│   │   └── config.toml
│   ├── Cargo.toml
│   ├── Cargo.lock
│   └── src/
│       └── sleep.rs
└── c/
    ├── Makefile (or CMakeLists.txt)
    └── src/
        └── sleep.c

Thanks for the informative YouTube video regarding this topic, I found it quite nice.

- Consolidated `use` statements and added `#![warn(clippy::all)]` for stricter linting
- Added compile-time check for Linux x86_64 targets.
- Use `hlt` instruction instead of empty loops to avoid wasting CPU.
- Replaced `as _` casts with explicit ones for clarity
- Improved `main_impl` error messages to better mimic GNU sleep
- Apply `rustfmt` to the code.
Per `execve(2)`, `argv` is an array of string pointers terminated by NULL.
Individual entries `argv[0]..argv[argc-1]` are guaranteed non-null, only the terminator `argv[argc]` is NULL.
We trust this kernel guarantee without runtime checks, so `NonNull` provides no safety/optimization benefit.

Source: https://man7.org/linux/man-pages/man2/execve.2.html
SYSCALL masks RFLAGS on entry (via IA32_FMASK) and restores from R11 on exit,
so flags are not preserved. Using `preserves_flags` is wrong.

Source: https://www.felixcloutier.com/x86/syscall
Since these are used in never-return paths it doesn't matter.
Also, more consistent with other `asm!` blocks.
- runtime/ABI glue (panic handler, personality)
- raw syscall primitives
- small utility helpers
- error reporting policy
- pure program logic (main_impl)
- ABI-facing main and _start
@valignatev
Copy link
Owner

what in the AI slop is this shit

@Big-Iron-Cheems
Copy link
Author

Big-Iron-Cheems commented Jan 4, 2026

If my way of writing longer than average commit messages comes off as LLM slop there's nothing I can do about that I'm afraid.
Personally I prefer clarity over brevity.

If this is about me linking sources in some commits, it's because for some changes I had to look at actual docs since I wasn't familiar with some of the ASM flags used here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants