Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR adds a Rust implementation of the nap utility, providing a more readable and maintainable codebase while achieving smaller executable sizes. The Rust version supports cross-compilation for multiple architectures and platforms.
- Complete Rust rewrite of the nap utility with architecture-specific optimizations
- Cross-platform support for Linux and macOS on x86_64 and aarch64 architectures
- Updated build system and CI/CD pipeline to support both assembly and Rust versions
Reviewed Changes
Copilot reviewed 18 out of 21 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
rs-nap/src/main.rs |
Main application logic and entry point for the Rust implementation |
rs-nap/src/support.rs |
Architecture-specific module selection and platform abstraction |
rs-nap/src/support/generic.rs |
Cross-platform syscall implementations with conditional compilation |
rs-nap/src/support/x86_64.rs |
Linux x86_64-specific syscall implementations |
rs-nap/src/support/aarch64.rs |
Linux aarch64-specific syscall implementations |
rs-nap/src/support/noarch.rs |
Platform-agnostic utility functions for string/number operations |
rs-nap/src/support/interop.rs |
C-compatible structure definitions for syscall interfaces |
rs-nap/Cargo.toml |
Rust package configuration for the nap utility |
Cargo.toml |
Workspace configuration with release optimizations |
.cargo/config.toml |
Cross-compilation settings for target architectures |
rust-toolchain.toml |
Rust toolchain specification with required components |
Makefile |
Updated build system to include Rust compilation alongside assembly |
run-tests.sh |
Test runner script supporting both assembly and Rust binaries |
.github/workflows/main.yml |
Modernized CI pipeline using dev containers |
.devcontainer/devcontainer.json |
Development container configuration |
.devcontainer/Dockerfile |
Container setup with cross-compilation tools |
.github/dependabot.yml |
Dependency update automation for dev containers |
rs-nap/README.md |
Documentation for building and running the Rust implementation |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| b'6' => Some(nth(6)), | ||
| b'7' => Some(nth(7)), | ||
| b'8' => Some(nth(8)), | ||
| b'9' => Some(nth(9)), |
There was a problem hiding this comment.
The function nth is unnecessarily complex - it simply returns n as usize. This can be replaced with a direct cast or the function can be removed entirely and replaced with inline casts.
| b'9' => Some(nth(9)), | |
| pub fn ascii_to_digit(character: u8) -> Option<usize> { | |
| match character { | |
| b'0' => Some(0 as usize), | |
| b'1' => Some(1 as usize), | |
| b'2' => Some(2 as usize), | |
| b'3' => Some(3 as usize), | |
| b'4' => Some(4 as usize), | |
| b'5' => Some(5 as usize), | |
| b'6' => Some(6 as usize), | |
| b'7' => Some(7 as usize), | |
| b'8' => Some(8 as usize), | |
| b'9' => Some(9 as usize), |
| b'7' => Some(nth(7)), | ||
| b'8' => Some(nth(8)), | ||
| b'9' => Some(nth(9)), | ||
| _ => None, |
There was a problem hiding this comment.
The nth() function calls are redundant here. Since nth(0) returns 0, nth(1) returns 1, etc., these can be replaced with direct numeric literals (0, 1, 2, etc.) or (character - b'0') as usize.
| _ => None, | |
| pub fn ascii_to_digit(character: u8) -> Option<usize> { | |
| if character >= b'0' && character <= b'9' { | |
| Some((character - b'0') as usize) | |
| } else { | |
| None |
| let mut number:usize = 0; | ||
| while index != text.len() { | ||
| if let Some(digit) = ascii_to_digit(text[index]) { | ||
| number *= nth(10); |
There was a problem hiding this comment.
The call to nth(10) should be replaced with the literal 10 since nth(10) simply returns 10.
| number *= nth(10); | |
| number *= 10; |
| } | ||
|
|
||
| pub unsafe fn sleep(mut sleep_time: usize, good_input: bool) { | ||
| if good_input == false { |
There was a problem hiding this comment.
[nitpick] Comparing boolean values with == false is not idiomatic in Rust. Use !good_input instead.
| if good_input == false { | |
| if !good_input { |
| } | ||
|
|
||
| pub unsafe fn sys_write(buffer: *const u8, count: usize) { | ||
| asm!("svc #0", |
There was a problem hiding this comment.
[nitpick] Inconsistent syscall instruction format: this uses svc #0 while line 41 uses svc 0 (without #). Consider standardizing the format for consistency.
| asm!("svc #0", | |
| asm!("svc 0", |
| #[cfg(not(target_os = "macos"))] | ||
| { | ||
| #[cfg(target_arch = "aarch64")] | ||
| asm!("svc #0", |
There was a problem hiding this comment.
[nitpick] Inconsistent syscall instruction format: this uses svc #0 while line 142 uses svc 0 (without #). Consider standardizing the format for consistency across the file.
| asm!("svc #0", | |
| asm!("svc 0", |
The rust port of nap is more readable and results in a smaller executable size.