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.
- 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:
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
const FLAGS_MASK: u8 = 0b0000_1111; // isolate lower 4 bits
fn extract_flags(byte: u8) -> u8 {
byte & FLAGS_MASK
}
</example>
<example id="2" title="Library Error Type (Ch 8)">
```rust
#[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) }
}
impl ConnectionState {
fn connect(addr: std::net::SocketAddr) -> Result<Self, AppError> {
let stream = std::net::TcpStream::connect(addr)?;
Ok(ConnectionState::Connected { stream })
}
}
</example>
<example id="4" title="Thread Pool Pattern (Ch 10)">
```rust
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");
}
}
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.
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
Anti-Patterns to Always Flag
static mut — a data race waiting to happen in concurrent code; replace with Arc<Mutex<T>> or an atomic type (Ch 6, 10)
from_ne_bytes / to_ne_bytes in protocols — native endianness is host-dependent; always use from_le_bytes/from_be_bytes (Ch 5, 7)
.unwrap() in library or I/O code — panics on error; use ? with Result propagation (Ch 3, 8)
Box<Vec<T>> — Vec<T> already heap-allocates; wrapping it in Box adds a pointless double-indirection; return Vec<T> directly (Ch 6)
Rc<T> passed to thread::spawn — Rc is not Send; use Arc<T> for shared ownership across threads (Ch 6)
- Indexing slices without bounds checks —
bytes[0..4] panics on short input; use bytes.get(0..4).ok_or(...) (Ch 5, 7)
- Not returning
JoinHandle from thread-spawning functions — callers cannot join threads or detect panics; return thread::JoinHandle<()> (Ch 10)
When reviewing Rust code:
- Always identify all endianness issues —
from_ne_bytes/to_ne_bytes in network or file contexts is always wrong.
- Always flag
static mut — it is undefined behavior in concurrent contexts; replace with atomics or Arc<Mutex<T>>.
- Always flag
.unwrap() in non-test I/O or library code — use ? and Result.
- Flag
Box<Vec<T>> as redundant double-indirection.
- Flag
Rc<T> used with thread::spawn — it will not compile, but explain why and offer Arc<T>.
- Flag unchecked slice indexing — suggest
.get(range).ok_or(...).
- Flag thread-spawning functions that discard the
JoinHandle.
- Recognize and praise:
Arc<Mutex<T>>, BufReader, AtomicU64 with Ordering::Relaxed, &Path parameters, custom error enums, .expect("mutex poisoned"), Arc::clone(&ptr), move closures.
- Always provide corrected code with comments explaining each change.
Source: booklib-ai/booklib — distributed by TomeVault.
1---2name: booklib-ai-booklib-rust-in-action3description: Rust in Action Skill4---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.3233<core_principles>34Check these areas in order of severity:35361. **Ownership & Borrowing** (Ch 4): Unnecessary `.clone()`? Value moved when a borrow would suffice? Use references where full ownership is not required.372. **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.383. **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.394. **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.405. **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.416. **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.427. **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.438. **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.449. **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).4510. **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.46</core_principles>4748### Step 3: Report Findings49For each issue, report:50- **Chapter reference** (e.g., "Ch 6: Smart Pointers")51- **Location** in the code52- **What's wrong** (the anti-pattern)53- **How to fix it** (the idiomatic / systems-correct approach)54- **Priority**: Critical (safety/UB/data corruption), Important (idiom/correctness), Suggestion (polish)5556### Step 4: Provide Fixed Code57Offer a corrected version with comments explaining each change.5859---6061## Mode 2: Writing New Code6263When the user asks you to **write** new Rust code, apply these core principles:6465<core_principles>66### Language Foundations (Ch 2)67681. **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.69702. **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.71723. **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.7374### Compound Types & Traits (Ch 3)75764. **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.77785. **Implement `new()` as the canonical constructor** (Ch 3). `impl MyStruct { pub fn new(...) -> Self { ... } }`. Use `Default` for zero-value construction.79806. **Implement `std::fmt::Display` for user-facing output, `Debug` via derive** (Ch 3). Derive `Debug`; hand-implement `Display`. Never use `{:?}` in user-facing messages.81827. **Use `pub(crate)` to limit visibility to the crate; keep internals private** (Ch 3). Public API surface should be minimal and intentional.83848. **Document public items with `///` rustdoc comments** (Ch 3). Include examples in doc comments — `cargo test` runs them.8586### Ownership, Borrowing & Smart Pointers (Ch 4, 6)87889. **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).899010. **Choose smart pointers by use case** (Ch 6):91 - `Box<T>` — heap allocation, single owner, unknown size at compile time92 - `Rc<T>` — shared ownership, single-threaded93 - `Arc<T>` — shared ownership, multi-threaded94 - `Cell<T>` — interior mutability for `Copy` types, single-threaded95 - `RefCell<T>` — interior mutability for non-`Copy`, single-threaded, runtime borrow checks96 - `Cow<'a, T>` — clone-on-write, avoids allocation when data is only read97 - `Arc<Mutex<T>>` — shared mutable state across threads989911. **Never use `Rc` across thread boundaries** (Ch 6). The compiler enforces this — `Rc` is not `Send`. Use `Arc` instead.10010112. **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.102103### Data Representation (Ch 5)10410513. **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.10610714. **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`.10810915. **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.110111### Files & Storage (Ch 7)11211316. **Use `BufReader`/`BufWriter` for file I/O** (Ch 7). Raw `File::read()` makes a syscall per call. `BufReader` batches reads into user-space buffer.11411517. **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.11611718. **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()`.118119### Networking (Ch 8)12012119. **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.12212320. **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.12412521. **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.126127### Concurrency (Ch 10)12812922. **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.13013123. **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.13213324. **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).13413525. **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.136137### Time (Ch 9)13813926. **Use `Instant` for elapsed time, `SystemTime` for wall clock** (Ch 9). `SystemTime` can go backwards (NTP adjustments, leap seconds). `Instant` is monotonic.14014127. **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.142</core_principles>143144---145146## Smart Pointer Selection Guide (Ch 6)147148```149Is the data shared across threads?150├── Yes → Arc<T> (read-only) or Arc<Mutex<T>> (mutable)151└── No152 ├── Shared (multiple owners, single thread)?153 │ └── Rc<T> (read-only) or Rc<RefCell<T>> (mutable)154 └── Single owner155 ├── Size unknown at compile time / recursive type?156 │ └── Box<T>157 ├── Usually read, occasionally cloned/modified?158 │ └── Cow<'a, T>159 └── Interior mutability needed?160 ├── Copy type → Cell<T>161 └── Non-Copy → RefCell<T>162```163164---165166## Code Structure Templates167168<examples>169<example id="1" title="Binary Protocol Field (Ch 5, 7)">170```rust171/// Parse a 4-byte big-endian u32 from a byte buffer at offset.172fn read_u32_be(buf: &[u8], offset: usize) -> Result<u32, ParseError> {173 buf.get(offset..offset + 4)174 .ok_or(ParseError::UnexpectedEof)175 .map(|b| u32::from_be_bytes(b.try_into().unwrap()))176}177178const FLAGS_MASK: u8 = 0b0000_1111; // isolate lower 4 bits179fn extract_flags(byte: u8) -> u8 {180 byte & FLAGS_MASK181}182```183</example>184185<example id="2" title="Library Error Type (Ch 8)">186```rust187#[derive(Debug)]188pub enum AppError {189 Io(std::io::Error),190 Network(std::net::AddrParseError),191 Parse(String),192}193194impl std::fmt::Display for AppError {195 fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {196 match self {197 AppError::Io(e) => write!(f, "I/O error: {e}"),198 AppError::Network(e) => write!(f, "network error: {e}"),199 AppError::Parse(msg) => write!(f, "parse error: {msg}"),200 }201 }202}203204impl std::error::Error for AppError {}205impl From<std::io::Error> for AppError {206 fn from(e: std::io::Error) -> Self { AppError::Io(e) }207}208impl From<std::net::AddrParseError> for AppError {209 fn from(e: std::net::AddrParseError) -> Self { AppError::Network(e) }210}211```212</example>213214<example id="3" title="State Machine with Enum (Ch 8)">215```rust216#[derive(Debug, Clone, PartialEq)]217enum ConnectionState {218 Idle,219 Connecting { addr: std::net::SocketAddr },220 Connected { stream: std::net::TcpStream },221 Closed,222}223224impl ConnectionState {225 fn connect(addr: std::net::SocketAddr) -> Result<Self, AppError> {226 let stream = std::net::TcpStream::connect(addr)?;227 Ok(ConnectionState::Connected { stream })228 }229}230```231</example>232233<example id="4" title="Thread Pool Pattern (Ch 10)">234```rust235use std::sync::{Arc, Mutex};236use std::sync::mpsc;237use std::thread;238239type Job = Box<dyn FnOnce() + Send + 'static>;240241struct ThreadPool {242 sender: mpsc::Sender<Job>,243}244245impl ThreadPool {246 fn new(size: usize) -> Self {247 let (sender, receiver) = mpsc::channel::<Job>();248 let receiver = Arc::new(Mutex::new(receiver));249250 for _ in 0..size {251 let rx = Arc::clone(&receiver);252 thread::spawn(move || loop {253 let job = rx.lock().expect("mutex poisoned").recv();254 match job {255 Ok(f) => f(),256 Err(_) => break, // channel closed257 }258 });259 }260 ThreadPool { sender }261 }262263 fn execute(&self, f: impl FnOnce() + Send + 'static) {264 self.sender.send(Box::new(f)).expect("thread pool closed");265 }266}267```268</example>269</examples>270271---272273<strengths_to_praise>274## Idiomatic Patterns — Do NOT Flag as Issues275276When reviewing code, recognize these patterns as **correct and idiomatic**. Do not manufacture issues from them:277278- **`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.279- **`BufReader<File>`** — explicitly recommended for batched I/O (Ch 7). Never call it overhead-adding or harmful.280- **`AtomicU64` with `Ordering::Relaxed`** — appropriate for counters where exact cross-thread ordering is not required, e.g. telemetry, stats (Ch 10). Not a bug.281- **`&Path` parameters** — correct; prefer `&Path` over `&str` or `String` for file paths (Ch 7).282- **Custom error enums with `Display` + `Error` + `From` impls** — the canonical library error pattern (Ch 3, 8). Praise it, don't critique it.283- **`.expect("mutex poisoned")` with a descriptive reason** — correct idiom for panicking on a poisoned mutex (Ch 10).284- **`Arc::clone(&ptr)` idiom** — correct; makes cheap refcount increment explicit (Ch 10).285- **`move` closures for threads** — required and correct (Ch 10).286287If 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.288</strengths_to_praise>289290---291292<anti_patterns>293## Priority of Practices by Impact294295### Critical (Safety, Correctness, UB)296- Ch 4: Borrow, don't clone — never move when a reference suffices297- Ch 6: Choose the right smart pointer — `Rc` is not thread-safe; don't share across threads298- Ch 6: Wrap `unsafe` in safe abstractions — document safety contracts with `// SAFETY:`299- Ch 5/7: Explicit endianness — wrong byte order silently corrupts binary data300- Ch 9: Use `Instant` for elapsed time — `SystemTime` can go backwards301302### Important (Idiom & Maintainability)303- Ch 3: Domain enums over stringly-typed state — invalid states should not compile304- Ch 8: Wrap downstream errors in a domain error type with `From` impls305- Ch 8: `impl Trait` over `dyn Trait` when types are homogeneous306- Ch 10: `move` closures for threads — required when closure outlives the stack frame307- Ch 10: `Arc::clone()` idiom — makes cheap pointer clone explicit308309### Suggestions (Systems Polish)310- Ch 2: Size integer types to the protocol spec — `u8` for bytes, `u16` for ports311- Ch 5: Named bit-mask constants — `const SIGN_BIT: u32 = 0x8000_0000`312- Ch 7: `BufReader`/`BufWriter` for all file I/O — syscall batching313- Ch 7: Checksums on binary writes — detect corruption on read314- Ch 9: NTP epoch offset constant — `const NTP_UNIX_OFFSET: u64 = 2_208_988_800`315316### Anti-Patterns to Always Flag317- **`static mut`** — a data race waiting to happen in concurrent code; replace with `Arc<Mutex<T>>` or an atomic type (Ch 6, 10)318- **`from_ne_bytes` / `to_ne_bytes` in protocols** — native endianness is host-dependent; always use `from_le_bytes`/`from_be_bytes` (Ch 5, 7)319- **`.unwrap()` in library or I/O code** — panics on error; use `?` with `Result` propagation (Ch 3, 8)320- **`Box<Vec<T>>`** — `Vec<T>` already heap-allocates; wrapping it in `Box` adds a pointless double-indirection; return `Vec<T>` directly (Ch 6)321- **`Rc<T>` passed to `thread::spawn`** — `Rc` is not `Send`; use `Arc<T>` for shared ownership across threads (Ch 6)322- **Indexing slices without bounds checks** — `bytes[0..4]` panics on short input; use `bytes.get(0..4).ok_or(...)` (Ch 5, 7)323- **Not returning `JoinHandle` from thread-spawning functions** — callers cannot join threads or detect panics; return `thread::JoinHandle<()>` (Ch 10)324</anti_patterns>325326<guidelines>327## Review Guidelines Summary328329When reviewing Rust code:3301. Always identify all endianness issues — `from_ne_bytes`/`to_ne_bytes` in network or file contexts is always wrong.3312. Always flag `static mut` — it is undefined behavior in concurrent contexts; replace with atomics or `Arc<Mutex<T>>`.3323. Always flag `.unwrap()` in non-test I/O or library code — use `?` and `Result`.3334. Flag `Box<Vec<T>>` as redundant double-indirection.3345. Flag `Rc<T>` used with `thread::spawn` — it will not compile, but explain why and offer `Arc<T>`.3356. Flag unchecked slice indexing — suggest `.get(range).ok_or(...)`.3367. Flag thread-spawning functions that discard the `JoinHandle`.3378. Recognize and praise: `Arc<Mutex<T>>`, `BufReader`, `AtomicU64` with `Ordering::Relaxed`, `&Path` parameters, custom error enums, `.expect("mutex poisoned")`, `Arc::clone(&ptr)`, `move` closures.3389. Always provide corrected code with comments explaining each change.339</guidelines>340341---342> Source: [booklib-ai/booklib](https://github.com/booklib-ai/booklib) — distributed by [TomeVault](https://tomevault.io).343<!-- tomevault:4.0:skill_md:2026-06-16 -->