Rust in Action Skill
Apply the systems programming practices from Tim McNamara's "Rust in Action" to review existing code and write new Rust. This skill operates in two modes: Review Mode (analyze code for violations of Rust idioms and systems programming correctness) and Write Mode (produce safe, idiomatic, systems-capable Rust from scratch).
The key differentiator of this book: Rust is taught through real systems — a CPU simulator, key-value store, NTP client, raw TCP stack, and OS kernel. Practices focus on correctness at the hardware boundary, not just language syntax.
Reference Files
practices-catalog.md — Before/after examples for ownership, smart pointers, bit ops, I/O, networking, concurrency, error wrapping, and state machines
How to Use This Skill
Before responding, read practices-catalog.md for the topic at hand. For ownership/borrowing issues read the ownership section. For systems/binary data read the data section. For a full review, read all sections.
Mode 1: Code Review
When the user asks you to review Rust code, follow this process:
Step 1: Identify the Domain
Determine whether the code is application-level, systems-level (binary data, I/O, networking, memory), or concurrent. The review focus shifts accordingly.
Step 2: Analyze the Code
Critical rule: Only flag genuine issues. If a pattern is idiomatic Rust, acknowledge it as correct. Do not manufacture problems where none exist. When code is well-written, say so and offer only minor suggestions. See the "Idiomatic Patterns — Do NOT Flag as Issues" section for patterns that must never be flagged.
Check these areas in order of severity:
- Ownership & Borrowing (Ch 4): Unnecessary
.clone()? Value moved when a borrow would suffice? Use references where full ownership is not required.
- Smart Pointer Choice (Ch 6): Is the right pointer type used?
Box<T> for heap, Rc<T> for single-thread shared, Arc<T> for multi-thread shared, RefCell<T> for interior mutability (single-thread), Mutex<T> for interior mutability (multi-thread). Cow<T> when data is usually read but occasionally mutated.
- Error Handling (Ch 3, 8):
.unwrap() or .expect() where ? belongs? For library code, define a custom error type that wraps downstream errors via From impl. Never leak internal error types across the public API boundary.
- Binary Data & Endianness (Ch 5, 7): Are integer byte representations explicit? Use
to_le_bytes() / from_le_bytes() / to_be_bytes(). Validate with checksums when writing binary formats. Use serde + bincode for structured serialization.
- Memory (Ch 6): Is
unsafe minimized? Raw pointer use must be bounded by a safe abstraction. Stack vs heap allocation: prefer stack; use Box only when size is unknown at compile time or you need heap lifetime.
- File & I/O (Ch 7): Use
BufReader/BufWriter for large files. Handle ENOENT, EPERM, ENOSPC distinctly — don't collapse I/O errors to strings. Use std::fs::Path for type-safe path handling.
- Networking (Ch 8): TCP state is implicit in OS — model explicit state machines with enums. Use trait objects (
Box<dyn Trait>) only when heterogeneous runtime dispatch is needed. Prefer impl Trait for static dispatch.
- Concurrency (Ch 10): Closures passed to threads must be
'static or use move. Shared mutable state needs Arc<Mutex<T>>. Use channels for message passing over shared state. Thread pool patterns over spawning one thread per task.
- Time (Ch 9): Don't use
std::time::SystemTime for elapsed measurement — it can go backwards. Use std::time::Instant for durations. For network time, NTP requires epoch conversion (NTP epoch: 1900 vs Unix: 1970 — offset 70 years = 2_208_988_800 seconds).
- Idioms: Iterator adapters over manual loops.
for item in &collection not for i in 0..collection.len(). if let/while let for single-variant matching. Exhaustive match — no silent wildcard arms.
Step 3: Report Findings
For each issue, report:
- Chapter reference (e.g., "Ch 6: Smart Pointers")
- Location in the code
- What's wrong (the anti-pattern)
- How to fix it (the idiomatic / systems-correct approach)
- Priority: Critical (safety/UB/data corruption), Important (idiom/correctness), Suggestion (polish)
Step 4: Provide Fixed Code
Offer a corrected version with comments explaining each change.
Mode 2: Writing New Code
When the user asks you to write new Rust code, apply these core principles:
Language Foundations (Ch 2)
Use cargo, not rustc directly (Ch 2). cargo new, cargo build, cargo test, cargo doc. Add third-party crates via Cargo.toml — never manually link.
Prefer integer types that match the domain (Ch 2). Use u8 for bytes, u16/u32/u64 for protocol fields sized to spec, i64 for timestamps. Avoid default usize for domain values.
Use loop for retry/event loops; while for condition-driven; for for iteration (Ch 2). Never use loop { if cond { break } } where while cond {} is clearer.
Compound Types & Traits (Ch 3)
Model domain state with enums, not stringly-typed flags (Ch 3). Enums with data (enum Packet { Ack(u32), Data(Vec<u8>) }) replace boolean + optional pairs and make invalid states unrepresentable.
Implement new() as the canonical constructor (Ch 3). impl MyStruct { pub fn new(...) -> Self { ... } }. Use Default for zero-value construction.
Implement std::fmt::Display for user-facing output, Debug via derive (Ch 3). Derive Debug; hand-implement Display. Never use {:?} in user-facing messages.
Use pub(crate) to limit visibility to the crate; keep internals private (Ch 3). Public API surface should be minimal and intentional.
Document public items with /// rustdoc comments (Ch 3). Include examples in doc comments — cargo test runs them.
Ownership, Borrowing & Smart Pointers (Ch 4, 6)
Use references where full ownership is not required (Ch 4). Pass &T for read, &mut T for write. Only transfer ownership when the callee must own (e.g., storing in a struct).
Choose smart pointers by use case (Ch 6):
Box<T> — heap allocation, single owner, unknown size at compile time
Rc<T> — shared ownership, single-threaded
Arc<T> — shared ownership, multi-threaded
Cell<T> — interior mutability for Copy types, single-threaded
RefCell<T> — interior mutability for non-Copy, single-threaded, runtime borrow checks
Cow<'a, T> — clone-on-write, avoids allocation when data is only read
Arc<Mutex<T>> — shared mutable state across threads
Never use Rc across thread boundaries (Ch 6). The compiler enforces this — Rc is not Send. Use Arc instead.
Minimize unsafe blocks; wrap them in safe abstractions (Ch 6). Raw pointers (*const T, *mut T) must be bounded within a module or function that upholds safety invariants. Document the safety contract with // SAFETY: comments.
Data Representation (Ch 5)
Be explicit about endianness in binary protocols (Ch 5, 7). Use u32::to_le_bytes(), u32::from_be_bytes() etc. Never assume native endianness when writing to disk or network.
Use bit operations to inspect and build packed data (Ch 5). AND (&) to isolate bits, OR (|) to set bits, shift (<<, >>) to position. Use named constants for masks: const SIGN_BIT: u32 = 0x8000_0000.
Validate binary data with checksums (Ch 7). For key-value stores and file formats, store a CRC or hash alongside data. Verify on read before trusting.
Files & Storage (Ch 7)
Use BufReader/BufWriter for file I/O (Ch 7). Raw File::read() makes a syscall per call. BufReader batches reads into user-space buffer.
Use serde + bincode for binary serialization (Ch 7). Add #[derive(Serialize, Deserialize)]; let bincode::serialize/deserialize handle encoding. Use serde_json for human-readable formats.
Use std::path::Path and PathBuf for file paths (Ch 7). Never build paths with string concatenation. Use path.join(), path.extension(), path.file_name().
Networking (Ch 8)
Model protocol state explicitly with enums (Ch 8). A TCP connection has states (SYN_SENT, ESTABLISHED, CLOSE_WAIT, etc.). Encode them as enum variants — the compiler enforces valid transitions.
Wrap library errors in a domain error type (Ch 8). When a function calls multiple libraries (network + I/O + parse), define an enum that wraps each. Implement From<LibError> for DomainError so ? converts automatically.
Use trait objects only for heterogeneous runtime dispatch (Ch 8). Vec<Box<dyn Animal>> is correct when you have a mixed collection. For a single concrete type, impl Trait is zero-cost.
Concurrency (Ch 10)
Use move closures when passing to threads (Ch 10). thread::spawn(move || { ... }) transfers ownership of captured variables into the thread. This is required when the closure outlives the current stack frame.
Use Arc::clone() explicitly, not .clone() on a value (Ch 10). Arc::clone(&ptr) is idiomatic — it's cheap (increments a reference count). Avoid .clone() on the inner value.
Use channels for work distribution; Arc<Mutex<T>> for shared state (Ch 10). Channels (std::sync::mpsc) are simpler and safer. Use shared state only when channels don't fit (e.g., result collection).
Use thread pools over raw thread::spawn per task (Ch 10). Spawning one thread per request doesn't scale. Use rayon, tokio, or a manual pool with a bounded queue.
Time (Ch 9)
Use Instant for elapsed time, SystemTime for wall clock (Ch 9). SystemTime can go backwards (NTP adjustments, leap seconds). Instant is monotonic.
Apply the NTP epoch offset when working with network time (Ch 9). NTP timestamps count seconds from 1900-01-01; Unix timestamps count from 1970-01-01. Offset: 2_208_988_800u64 seconds.
Smart Pointer Selection Guide (Ch 6)
Is the data shared across threads?
├── Yes → Arc<T> (read-only) or Arc<Mutex<T>> (mutable)
└── No
├── Shared (multiple owners, single thread)?
│ └── Rc<T> (read-only) or Rc<RefCell<T>> (mutable)
└── Single owner
├── Size unknown at compile time / recursive type?
│ └── Box<T>
├── Usually read, occasionally cloned/modified?
│ └── Cow<'a, T>
└── Interior mutability needed?
├── Copy type → Cell<T>
└── Non-Copy → RefCell<T>
Code Structure Templates
Binary Protocol Field (Ch 5, 7)
/// Parse a 4-byte big-endian u32 from a byte buffer at offset.
fn read_u32_be(buf: &[u8], offset: usize) -> Result<u32, ParseError> {
buf.get(offset..offset + 4)
.ok_or(ParseError::UnexpectedEof)
.map(|b| u32::from_be_bytes(b.try_into().unwrap()))
}
const FLAGS_MASK: u8 = 0b0000_1111; // isolate lower 4 bits
fn extract_flags(byte: u8) -> u8 {
byte & FLAGS_MASK
}
Library Error Type (Ch 8)
#[derive(Debug)]
pub enum AppError {
Io(std::io::Error),
Network(std::net::AddrParseError),
Parse(String),
}
impl std::fmt::Display for AppError {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
match self {
AppError::Io(e) => write!(f, "I/O error: {e}"),
AppError::Network(e) => write!(f, "network error: {e}"),
AppError::Parse(msg) => write!(f, "parse error: {msg}"),
}
}
}
impl std::error::Error for AppError {}
impl From<std::io::Error> for AppError {
fn from(e: std::io::Error) -> Self { AppError::Io(e) }
}
impl From<std::net::AddrParseError> for AppError {
fn from(e: std::net::AddrParseError) -> Self { AppError::Network(e) }
}
State Machine with Enum (Ch 8)
#[derive(Debug, Clone, PartialEq)]
enum ConnectionState {
Idle,
Connecting { addr: std::net::SocketAddr },
Connected { stream: std::net::TcpStream },
Closed,
}
impl ConnectionState {
fn connect(addr: std::net::SocketAddr) -> Result<Self, AppError> {
let stream = std::net::TcpStream::connect(addr)?;
Ok(ConnectionState::Connected { stream })
}
}
Thread Pool Pattern (Ch 10)
use std::sync::{Arc, Mutex};
use std::sync::mpsc;
use std::thread;
type Job = Box<dyn FnOnce() + Send + 'static>;
struct ThreadPool {
sender: mpsc::Sender<Job>,
}
impl ThreadPool {
fn new(size: usize) -> Self {
let (sender, receiver) = mpsc::channel::<Job>();
let receiver = Arc::new(Mutex::new(receiver));
for _ in 0..size {
let rx = Arc::clone(&receiver);
thread::spawn(move || loop {
let job = rx.lock().expect("mutex poisoned").recv();
match job {
Ok(f) => f(),
Err(_) => break, // channel closed
}
});
}
ThreadPool { sender }
}
fn execute(&self, f: impl FnOnce() + Send + 'static) {
self.sender.send(Box::new(f)).expect("thread pool closed");
}
}
Idiomatic Patterns — Do NOT Flag as Issues
When reviewing code, recognize these patterns as correct and idiomatic. Do not manufacture issues from them:
Arc<Mutex<T>> — the standard pattern for shared mutable state across threads (Ch 6, 10). This is correct and idiomatic. Never call it redundant, a design error, or suggest removing/replacing it when it is the appropriate choice. When a Store or similar struct wraps shared mutable data accessed by multiple threads, Arc<Mutex<Vec<u8>>> is exactly right — acknowledge it as such.
BufReader<File> — explicitly recommended for batched I/O (Ch 7). Never call it overhead-adding or harmful.
AtomicU64 with Ordering::Relaxed — appropriate for counters where exact cross-thread ordering is not required, e.g. telemetry, stats (Ch 10). Not a bug.
&Path parameters — correct; prefer &Path over &str or String for file paths (Ch 7).
- Custom error enums with
Display + Error + From impls — the canonical library error pattern (Ch 3, 8). Praise it, don't critique it.
.expect("mutex poisoned") with a descriptive reason — correct idiom for panicking on a poisoned mutex (Ch 10).
Arc::clone(&ptr) idiom — correct; makes cheap refcount increment explicit (Ch 10).
move closures for threads — required and correct (Ch 10).
If code is already correct, say so. Only flag real issues. If the only things left to say are minor suggestions, label them as suggestions, not bugs or important issues.
Priority of Practices by Impact
Critical (Safety, Correctness, UB)
- Ch 4: Borrow, don't clone — never move when a reference suffices
- Ch 6: Choose the right smart pointer —
Rc is not thread-safe; don't share across threads
- Ch 6: Wrap
unsafe in safe abstractions — document safety contracts with // SAFETY:
- Ch 5/7: Explicit endianness — wrong byte order silently corrupts binary data
- Ch 9: Use
Instant for elapsed time — SystemTime can go backwards
Important (Idiom & Maintainability)
- Ch 3: Domain enums over stringly-typed state — invalid states should not compile
- Ch 8: Wrap downstream errors in a domain error type with
From impls
- Ch 8:
impl Trait over dyn Trait when types are homogeneous
- Ch 10:
move closures for threads — required when closure outlives the stack frame
- Ch 10:
Arc::clone() idiom — makes cheap pointer clone explicit
Suggestions (Systems Polish)
- Ch 2: Size integer types to the protocol spec —
u8 for bytes, u16 for ports
- Ch 5: Named bit-mask constants —
const SIGN_BIT: u32 = 0x8000_0000
- Ch 7:
BufReader/BufWriter for all file I/O — syscall batching
- Ch 7: Checksums on binary writes — detect corruption on read
- Ch 9: NTP epoch offset constant —
const NTP_UNIX_OFFSET: u64 = 2_208_988_800
1---2name: rust-in-action3description: Write and review Rust code using systems programming concepts from "Rust in Action" by Tim McNamara. Covers language foundations, ownership and borrowing, smart pointers, data representation (bits, endianness, floats), memory (stack/heap/virtual), file I/O, networking (TCP/HTTP), concurrency (threads/closures), and OS-level programming. Use when writing systems-level Rust, working with binary data, raw pointers, file formats, network protocols, or low-level memory. Trigger on: "Rust", "systems programming", "smart pointers", "endianness", "bit manipulation", "TCP", "raw pointers", "serde", "borrow checker", "ownership", "concurrency", "kernel", ".rs files", "cargo".4---56# Rust in Action Skill78Apply the systems programming practices from Tim McNamara's "Rust in Action" to review existing code and write new Rust. This skill operates in two modes: **Review Mode** (analyze code for violations of Rust idioms and systems programming correctness) and **Write Mode** (produce safe, idiomatic, systems-capable Rust from scratch).910The key differentiator of this book: Rust is taught through real systems — a CPU simulator, key-value store, NTP client, raw TCP stack, and OS kernel. Practices focus on correctness at the hardware boundary, not just language syntax.1112## Reference Files1314- `practices-catalog.md` — Before/after examples for ownership, smart pointers, bit ops, I/O, networking, concurrency, error wrapping, and state machines1516## How to Use This Skill1718**Before responding**, read `practices-catalog.md` for the topic at hand. For ownership/borrowing issues read the ownership section. For systems/binary data read the data section. For a full review, read all sections.1920---2122## Mode 1: Code Review2324When the user asks you to **review** Rust code, follow this process:2526### Step 1: Identify the Domain27Determine whether the code is application-level, systems-level (binary data, I/O, networking, memory), or concurrent. The review focus shifts accordingly.2829### Step 2: Analyze the Code3031**Critical rule**: Only flag genuine issues. If a pattern is idiomatic Rust, acknowledge it as correct. Do not manufacture problems where none exist. When code is well-written, say so and offer only minor suggestions. See the "Idiomatic Patterns — Do NOT Flag as Issues" section for patterns that must never be flagged.3233Check these areas in order of severity:34351. **Ownership & Borrowing** (Ch 4): Unnecessary `.clone()`? Value moved when a borrow would suffice? Use references where full ownership is not required.362. **Smart Pointer Choice** (Ch 6): Is the right pointer type used? `Box<T>` for heap, `Rc<T>` for single-thread shared, `Arc<T>` for multi-thread shared, `RefCell<T>` for interior mutability (single-thread), `Mutex<T>` for interior mutability (multi-thread). `Cow<T>` when data is usually read but occasionally mutated.373. **Error Handling** (Ch 3, 8): `.unwrap()` or `.expect()` where `?` belongs? For library code, define a custom error type that wraps downstream errors via `From` impl. Never leak internal error types across the public API boundary.384. **Binary Data & Endianness** (Ch 5, 7): Are integer byte representations explicit? Use `to_le_bytes()` / `from_le_bytes()` / `to_be_bytes()`. Validate with checksums when writing binary formats. Use `serde` + `bincode` for structured serialization.395. **Memory** (Ch 6): Is `unsafe` minimized? Raw pointer use must be bounded by a safe abstraction. Stack vs heap allocation: prefer stack; use `Box` only when size is unknown at compile time or you need heap lifetime.406. **File & I/O** (Ch 7): Use `BufReader`/`BufWriter` for large files. Handle `ENOENT`, `EPERM`, `ENOSPC` distinctly — don't collapse I/O errors to strings. Use `std::fs::Path` for type-safe path handling.417. **Networking** (Ch 8): TCP state is implicit in OS — model explicit state machines with enums. Use trait objects (`Box<dyn Trait>`) only when heterogeneous runtime dispatch is needed. Prefer `impl Trait` for static dispatch.428. **Concurrency** (Ch 10): Closures passed to threads must be `'static` or use `move`. Shared mutable state needs `Arc<Mutex<T>>`. Use channels for message passing over shared state. Thread pool patterns over spawning one thread per task.439. **Time** (Ch 9): Don't use `std::time::SystemTime` for elapsed measurement — it can go backwards. Use `std::time::Instant` for durations. For network time, NTP requires epoch conversion (NTP epoch: 1900 vs Unix: 1970 — offset 70 years = 2_208_988_800 seconds).4410. **Idioms**: Iterator adapters over manual loops. `for item in &collection` not `for i in 0..collection.len()`. `if let`/`while let` for single-variant matching. Exhaustive `match` — no silent wildcard arms.4546### Step 3: Report Findings47For each issue, report:48- **Chapter reference** (e.g., "Ch 6: Smart Pointers")49- **Location** in the code50- **What's wrong** (the anti-pattern)51- **How to fix it** (the idiomatic / systems-correct approach)52- **Priority**: Critical (safety/UB/data corruption), Important (idiom/correctness), Suggestion (polish)5354### Step 4: Provide Fixed Code55Offer a corrected version with comments explaining each change.5657---5859## Mode 2: Writing New Code6061When the user asks you to **write** new Rust code, apply these core principles:6263### Language Foundations (Ch 2)64651. **Use cargo, not rustc directly** (Ch 2). `cargo new`, `cargo build`, `cargo test`, `cargo doc`. Add third-party crates via `Cargo.toml` — never manually link.66672. **Prefer integer types that match the domain** (Ch 2). Use `u8` for bytes, `u16`/`u32`/`u64` for protocol fields sized to spec, `i64` for timestamps. Avoid default `usize` for domain values.68693. **Use `loop` for retry/event loops; `while` for condition-driven; `for` for iteration** (Ch 2). Never use `loop { if cond { break } }` where `while cond {}` is clearer.7071### Compound Types & Traits (Ch 3)72734. **Model domain state with enums, not stringly-typed flags** (Ch 3). Enums with data (`enum Packet { Ack(u32), Data(Vec<u8>) }`) replace boolean + optional pairs and make invalid states unrepresentable.74755. **Implement `new()` as the canonical constructor** (Ch 3). `impl MyStruct { pub fn new(...) -> Self { ... } }`. Use `Default` for zero-value construction.76776. **Implement `std::fmt::Display` for user-facing output, `Debug` via derive** (Ch 3). Derive `Debug`; hand-implement `Display`. Never use `{:?}` in user-facing messages.78797. **Use `pub(crate)` to limit visibility to the crate; keep internals private** (Ch 3). Public API surface should be minimal and intentional.80818. **Document public items with `///` rustdoc comments** (Ch 3). Include examples in doc comments — `cargo test` runs them.8283### Ownership, Borrowing & Smart Pointers (Ch 4, 6)84859. **Use references where full ownership is not required** (Ch 4). Pass `&T` for read, `&mut T` for write. Only transfer ownership when the callee must own (e.g., storing in a struct).868710. **Choose smart pointers by use case** (Ch 6):88 - `Box<T>` — heap allocation, single owner, unknown size at compile time89 - `Rc<T>` — shared ownership, single-threaded90 - `Arc<T>` — shared ownership, multi-threaded91 - `Cell<T>` — interior mutability for `Copy` types, single-threaded92 - `RefCell<T>` — interior mutability for non-`Copy`, single-threaded, runtime borrow checks93 - `Cow<'a, T>` — clone-on-write, avoids allocation when data is only read94 - `Arc<Mutex<T>>` — shared mutable state across threads959611. **Never use `Rc` across thread boundaries** (Ch 6). The compiler enforces this — `Rc` is not `Send`. Use `Arc` instead.979812. **Minimize `unsafe` blocks; wrap them in safe abstractions** (Ch 6). Raw pointers (`*const T`, `*mut T`) must be bounded within a module or function that upholds safety invariants. Document the safety contract with `// SAFETY:` comments.99100### Data Representation (Ch 5)10110213. **Be explicit about endianness in binary protocols** (Ch 5, 7). Use `u32::to_le_bytes()`, `u32::from_be_bytes()` etc. Never assume native endianness when writing to disk or network.10310414. **Use bit operations to inspect and build packed data** (Ch 5). AND (`&`) to isolate bits, OR (`|`) to set bits, shift (`<<`, `>>`) to position. Use named constants for masks: `const SIGN_BIT: u32 = 0x8000_0000`.10510615. **Validate binary data with checksums** (Ch 7). For key-value stores and file formats, store a CRC or hash alongside data. Verify on read before trusting.107108### Files & Storage (Ch 7)10911016. **Use `BufReader`/`BufWriter` for file I/O** (Ch 7). Raw `File::read()` makes a syscall per call. `BufReader` batches reads into user-space buffer.11111217. **Use `serde` + `bincode` for binary serialization** (Ch 7). Add `#[derive(Serialize, Deserialize)]`; let `bincode::serialize`/`deserialize` handle encoding. Use `serde_json` for human-readable formats.11311418. **Use `std::path::Path` and `PathBuf` for file paths** (Ch 7). Never build paths with string concatenation. Use `path.join()`, `path.extension()`, `path.file_name()`.115116### Networking (Ch 8)11711819. **Model protocol state explicitly with enums** (Ch 8). A TCP connection has states (SYN_SENT, ESTABLISHED, CLOSE_WAIT, etc.). Encode them as enum variants — the compiler enforces valid transitions.11912020. **Wrap library errors in a domain error type** (Ch 8). When a function calls multiple libraries (network + I/O + parse), define an enum that wraps each. Implement `From<LibError> for DomainError` so `?` converts automatically.12112221. **Use trait objects only for heterogeneous runtime dispatch** (Ch 8). `Vec<Box<dyn Animal>>` is correct when you have a mixed collection. For a single concrete type, `impl Trait` is zero-cost.123124### Concurrency (Ch 10)12512622. **Use `move` closures when passing to threads** (Ch 10). `thread::spawn(move || { ... })` transfers ownership of captured variables into the thread. This is required when the closure outlives the current stack frame.12712823. **Use `Arc::clone()` explicitly, not `.clone()` on a value** (Ch 10). `Arc::clone(&ptr)` is idiomatic — it's cheap (increments a reference count). Avoid `.clone()` on the inner value.12913024. **Use channels for work distribution; `Arc<Mutex<T>>` for shared state** (Ch 10). Channels (`std::sync::mpsc`) are simpler and safer. Use shared state only when channels don't fit (e.g., result collection).13113225. **Use thread pools over raw `thread::spawn` per task** (Ch 10). Spawning one thread per request doesn't scale. Use `rayon`, `tokio`, or a manual pool with a bounded queue.133134### Time (Ch 9)13513626. **Use `Instant` for elapsed time, `SystemTime` for wall clock** (Ch 9). `SystemTime` can go backwards (NTP adjustments, leap seconds). `Instant` is monotonic.13713827. **Apply the NTP epoch offset when working with network time** (Ch 9). NTP timestamps count seconds from 1900-01-01; Unix timestamps count from 1970-01-01. Offset: `2_208_988_800u64` seconds.139140---141142## Smart Pointer Selection Guide (Ch 6)143144```145Is the data shared across threads?146├── Yes → Arc<T> (read-only) or Arc<Mutex<T>> (mutable)147└── No148 ├── Shared (multiple owners, single thread)?149 │ └── Rc<T> (read-only) or Rc<RefCell<T>> (mutable)150 └── Single owner151 ├── Size unknown at compile time / recursive type?152 │ └── Box<T>153 ├── Usually read, occasionally cloned/modified?154 │ └── Cow<'a, T>155 └── Interior mutability needed?156 ├── Copy type → Cell<T>157 └── Non-Copy → RefCell<T>158```159160---161162## Code Structure Templates163164### Binary Protocol Field (Ch 5, 7)165```rust166/// Parse a 4-byte big-endian u32 from a byte buffer at offset.167fn read_u32_be(buf: &[u8], offset: usize) -> Result<u32, ParseError> {168 buf.get(offset..offset + 4)169 .ok_or(ParseError::UnexpectedEof)170 .map(|b| u32::from_be_bytes(b.try_into().unwrap()))171}172173const FLAGS_MASK: u8 = 0b0000_1111; // isolate lower 4 bits174fn extract_flags(byte: u8) -> u8 {175 byte & FLAGS_MASK176}177```178179### Library Error Type (Ch 8)180```rust181#[derive(Debug)]182pub enum AppError {183 Io(std::io::Error),184 Network(std::net::AddrParseError),185 Parse(String),186}187188impl std::fmt::Display for AppError {189 fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {190 match self {191 AppError::Io(e) => write!(f, "I/O error: {e}"),192 AppError::Network(e) => write!(f, "network error: {e}"),193 AppError::Parse(msg) => write!(f, "parse error: {msg}"),194 }195 }196}197198impl std::error::Error for AppError {}199impl From<std::io::Error> for AppError {200 fn from(e: std::io::Error) -> Self { AppError::Io(e) }201}202impl From<std::net::AddrParseError> for AppError {203 fn from(e: std::net::AddrParseError) -> Self { AppError::Network(e) }204}205```206207### State Machine with Enum (Ch 8)208```rust209#[derive(Debug, Clone, PartialEq)]210enum ConnectionState {211 Idle,212 Connecting { addr: std::net::SocketAddr },213 Connected { stream: std::net::TcpStream },214 Closed,215}216217impl ConnectionState {218 fn connect(addr: std::net::SocketAddr) -> Result<Self, AppError> {219 let stream = std::net::TcpStream::connect(addr)?;220 Ok(ConnectionState::Connected { stream })221 }222}223```224225### Thread Pool Pattern (Ch 10)226```rust227use std::sync::{Arc, Mutex};228use std::sync::mpsc;229use std::thread;230231type Job = Box<dyn FnOnce() + Send + 'static>;232233struct ThreadPool {234 sender: mpsc::Sender<Job>,235}236237impl ThreadPool {238 fn new(size: usize) -> Self {239 let (sender, receiver) = mpsc::channel::<Job>();240 let receiver = Arc::new(Mutex::new(receiver));241242 for _ in 0..size {243 let rx = Arc::clone(&receiver);244 thread::spawn(move || loop {245 let job = rx.lock().expect("mutex poisoned").recv();246 match job {247 Ok(f) => f(),248 Err(_) => break, // channel closed249 }250 });251 }252 ThreadPool { sender }253 }254255 fn execute(&self, f: impl FnOnce() + Send + 'static) {256 self.sender.send(Box::new(f)).expect("thread pool closed");257 }258}259```260261---262263## Idiomatic Patterns — Do NOT Flag as Issues264265When reviewing code, recognize these patterns as **correct and idiomatic**. Do not manufacture issues from them:266267- **`Arc<Mutex<T>>`** — the standard pattern for shared mutable state across threads (Ch 6, 10). This is correct and idiomatic. Never call it redundant, a design error, or suggest removing/replacing it when it is the appropriate choice. When a `Store` or similar struct wraps shared mutable data accessed by multiple threads, `Arc<Mutex<Vec<u8>>>` is exactly right — acknowledge it as such.268- **`BufReader<File>`** — explicitly recommended for batched I/O (Ch 7). Never call it overhead-adding or harmful.269- **`AtomicU64` with `Ordering::Relaxed`** — appropriate for counters where exact cross-thread ordering is not required, e.g. telemetry, stats (Ch 10). Not a bug.270- **`&Path` parameters** — correct; prefer `&Path` over `&str` or `String` for file paths (Ch 7).271- **Custom error enums with `Display` + `Error` + `From` impls** — the canonical library error pattern (Ch 3, 8). Praise it, don't critique it.272- **`.expect("mutex poisoned")`** with a descriptive reason — correct idiom for panicking on a poisoned mutex (Ch 10).273- **`Arc::clone(&ptr)` idiom** — correct; makes cheap refcount increment explicit (Ch 10).274- **`move` closures for threads** — required and correct (Ch 10).275276If code is already correct, say so. Only flag real issues. If the only things left to say are minor suggestions, label them as suggestions, not bugs or important issues.277278---279280## Priority of Practices by Impact281282### Critical (Safety, Correctness, UB)283- Ch 4: Borrow, don't clone — never move when a reference suffices284- Ch 6: Choose the right smart pointer — `Rc` is not thread-safe; don't share across threads285- Ch 6: Wrap `unsafe` in safe abstractions — document safety contracts with `// SAFETY:`286- Ch 5/7: Explicit endianness — wrong byte order silently corrupts binary data287- Ch 9: Use `Instant` for elapsed time — `SystemTime` can go backwards288289### Important (Idiom & Maintainability)290- Ch 3: Domain enums over stringly-typed state — invalid states should not compile291- Ch 8: Wrap downstream errors in a domain error type with `From` impls292- Ch 8: `impl Trait` over `dyn Trait` when types are homogeneous293- Ch 10: `move` closures for threads — required when closure outlives the stack frame294- Ch 10: `Arc::clone()` idiom — makes cheap pointer clone explicit295296### Suggestions (Systems Polish)297- Ch 2: Size integer types to the protocol spec — `u8` for bytes, `u16` for ports298- Ch 5: Named bit-mask constants — `const SIGN_BIT: u32 = 0x8000_0000`299- Ch 7: `BufReader`/`BufWriter` for all file I/O — syscall batching300- Ch 7: Checksums on binary writes — detect corruption on read301- Ch 9: NTP epoch offset constant — `const NTP_UNIX_OFFSET: u64 = 2_208_988_800`