Skip to content

Conversation

@ca0abinary
Copy link
Contributor

The rust port of nap is more readable and results in a smaller executable size.

Copilot AI review requested due to automatic review settings August 14, 2025 13:13
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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)),
Copy link

Copilot AI Aug 14, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Suggested change
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),

Copilot uses AI. Check for mistakes.
b'7' => Some(nth(7)),
b'8' => Some(nth(8)),
b'9' => Some(nth(9)),
_ => None,
Copy link

Copilot AI Aug 14, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Suggested change
_ => 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

Copilot uses AI. Check for mistakes.
let mut number:usize = 0;
while index != text.len() {
if let Some(digit) = ascii_to_digit(text[index]) {
number *= nth(10);
Copy link

Copilot AI Aug 14, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The call to nth(10) should be replaced with the literal 10 since nth(10) simply returns 10.

Suggested change
number *= nth(10);
number *= 10;

Copilot uses AI. Check for mistakes.
}

pub unsafe fn sleep(mut sleep_time: usize, good_input: bool) {
if good_input == false {
Copy link

Copilot AI Aug 14, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[nitpick] Comparing boolean values with == false is not idiomatic in Rust. Use !good_input instead.

Suggested change
if good_input == false {
if !good_input {

Copilot uses AI. Check for mistakes.
}

pub unsafe fn sys_write(buffer: *const u8, count: usize) {
asm!("svc #0",
Copy link

Copilot AI Aug 14, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[nitpick] Inconsistent syscall instruction format: this uses svc #0 while line 41 uses svc 0 (without #). Consider standardizing the format for consistency.

Suggested change
asm!("svc #0",
asm!("svc 0",

Copilot uses AI. Check for mistakes.
#[cfg(not(target_os = "macos"))]
{
#[cfg(target_arch = "aarch64")]
asm!("svc #0",
Copy link

Copilot AI Aug 14, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[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.

Suggested change
asm!("svc #0",
asm!("svc 0",

Copilot uses AI. Check for mistakes.
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.

1 participant