docs: add comprehensive code quality issues and fixes plan
Co-authored-by: aider (openai/andrew/openrouter/anthropic/claude-sonnet-4) <aider@aider.chat>
This commit is contained in:
83
PLAN.md
83
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<Mutex<T>> 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
|
||||||
|
|||||||
Reference in New Issue
Block a user