From f88897000f9c532f44935d3f31f8b8ba8604eb9c Mon Sep 17 00:00:00 2001 From: Andrew Phillips Date: Sat, 9 Aug 2025 23:10:21 -0300 Subject: [PATCH] docs: add comprehensive code quality issues and fixes plan Co-authored-by: aider (openai/andrew/openrouter/anthropic/claude-sonnet-4) --- PLAN.md | 83 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 83 insertions(+) diff --git a/PLAN.md b/PLAN.md index e69de29..53148e0 100644 --- a/PLAN.md +++ b/PLAN.md @@ -0,0 +1,83 @@ +# Code Quality Issues and Fixes + +## Critical Issues + +### 1. Memory Safety & Resource Leaks +**Files affected:** `src/modes/diff.rs`, `src/compression_engine/program.rs` +**Functions affected:** `mode_diff()`, `CompressionEngineProgram::open()`, `CompressionEngineProgram::create()` +**Problem example:** Raw file descriptors converted with `unsafe { std::fs::File::from_raw_fd(fd_write) }` without proper cleanup on errors +**Fix example:** Use RAII wrappers or ensure proper cleanup in Drop implementations and error paths + +### 2. Error Handling Problems +**Files affected:** `src/modes/save.rs`, `src/modes/update.rs`, `src/db.rs` +**Functions affected:** `mode_save()`, `mode_update()`, `get_item()`, `insert_item()` +**Problem example:** `item.id.unwrap()` can panic if item.id is None +**Fix example:** Replace with `item.id.ok_or_else(|| anyhow!("Item missing ID"))?` + +### 3. Concurrency Issues +**Files affected:** `src/modes/diff.rs`, `src/meta_plugin/digest.rs` +**Functions affected:** `mode_diff()`, meta plugin `update()` methods +**Problem example:** Multiple threads accessing shared meta plugin state without synchronization +**Fix example:** Use Arc> for shared state or ensure thread-local plugin instances + +## Design Problems + +### 4. Database Design Issues +**Files affected:** `src/db.rs`, `src/modes/save.rs`, `src/modes/update.rs` +**Functions affected:** `insert_item()`, `update_item()`, `store_meta()`, `set_item_tags()` +**Problem example:** Multiple database operations without transactions can leave partial state +**Fix example:** Wrap related operations in `conn.transaction()` blocks + +### 5. Plugin Architecture Flaws +**Files affected:** `src/meta_plugin.rs`, `src/meta_plugin/digest.rs`, `src/meta_plugin/program.rs` +**Functions affected:** `MetaPlugin::create()`, `MetaPlugin::update()`, `MetaPlugin::finalize()` +**Problem example:** `create()` returns dummy writer that's never used, inconsistent with actual usage pattern +**Fix example:** Remove `create()` method and rely only on `update()`/`finalize()` pattern + +### 6. Security Concerns +**Files affected:** `src/main.rs`, `src/modes/get.rs`, `src/modes/delete.rs` +**Functions affected:** `main()`, `mode_get()`, `mode_delete()` +**Problem example:** Item IDs used directly in file paths without validation: `item_path.push(item_id.to_string())` +**Fix example:** Validate item IDs are positive integers and sanitize file paths + +## Performance Issues + +### 7. Inefficient Operations +**Files affected:** `src/modes/save.rs`, `src/compression_engine.rs` +**Functions affected:** `mode_save()`, `CompressionEngine::size()` +**Problem example:** Fixed 4KB buffer `let mut buffer = [0; libc::BUFSIZ as usize];` may be too small +**Fix example:** Use adaptive buffer sizing or larger default buffers (64KB+) + +### 8. I/O Problems +**Files affected:** `src/modes/save.rs`, `src/modes/get.rs`, `src/compression_engine.rs` +**Functions affected:** `mode_save()`, `mode_get()`, `CompressionEngine::cat()` +**Problem example:** All I/O operations are blocking with no progress indication +**Fix example:** Add progress callbacks or async I/O with tokio + +## Code Quality Issues + +### 9. Error Messages +**Files affected:** `src/modes/common.rs`, `src/main.rs` +**Functions affected:** `cmd_args_digest_type()`, `cmd_args_compression_type()`, `main()` +**Problem example:** `format!("Unknown digest type: {}", digest_name)` exposes internal terminology +**Fix example:** `format!("Invalid digest algorithm '{}'. Use 'sha256' or 'md5'", digest_name)` + +### 10. Code Organization +**Files affected:** `src/modes/save.rs`, `src/modes/diff.rs` +**Functions affected:** `mode_save()`, `mode_diff()` +**Problem example:** `mode_save()` function is 150+ lines doing I/O, meta processing, and database operations +**Fix example:** Split into smaller functions: `setup_compression()`, `process_meta_plugins()`, `save_to_database()` + +## Specific Bug Risks + +### 11. Race Conditions +**Files affected:** `src/db.rs`, `src/modes/save.rs` +**Functions affected:** `insert_item()`, `mode_save()` +**Problem example:** Multiple processes could create items with same auto-increment ID simultaneously +**Fix example:** Use database-level unique constraints and handle conflicts gracefully + +### 12. Data Integrity +**Files affected:** `src/modes/save.rs`, `src/modes/update.rs` +**Functions affected:** `mode_save()`, `mode_update()` +**Problem example:** Database entry created before file is fully written, crash could leave orphaned entries +**Fix example:** Write to temporary file first, then move to final location and update database atomically