chore: remove completed items from PLAN.md
This commit is contained in:
committed by
Andrew Phillips (aider)
parent
0eafdd0985
commit
027fa10f04
81
PLAN.md
81
PLAN.md
@@ -1,81 +0,0 @@
|
|||||||
# Code Quality Issues and Fixes
|
|
||||||
|
|
||||||
## Critical Issues
|
|
||||||
|
|
||||||
### 1. Memory Safety & Resource Leaks - DONE
|
|
||||||
**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 - DONE
|
|
||||||
**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 - DONE
|
|
||||||
**Files affected:** `src/modes/diff.rs`, `src/meta_plugin/digest.rs`
|
|
||||||
**Functions affected:** `mode_diff()`, meta plugin `update()` methods
|
|
||||||
**Problem example:** In `mode_diff()`, if writer threads panic, resources may not be cleaned up properly: `writer_thread_a.join()` only propagates panic but doesn't ensure file descriptors are closed
|
|
||||||
**Fix example:** Use RAII guards or ensure cleanup in panic handlers: `let _fd_guard = FileDescriptorGuard::new(fd_write);`
|
|
||||||
|
|
||||||
## Design Problems
|
|
||||||
|
|
||||||
### 4. Database Design Issues - DONE
|
|
||||||
**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
|
|
||||||
- `MetaPluginProgram::finalize()` spawns new process instead of reusing existing one
|
|
||||||
- No validation that meta plugins produce valid output formats
|
|
||||||
- Plugin errors are silently ignored in save operations
|
|
||||||
**Fix example:**
|
|
||||||
- Remove `create()` method and rely only on `update()`/`finalize()` pattern
|
|
||||||
- Reuse single process per plugin instance for better performance
|
|
||||||
- Add output validation and proper error propagation
|
|
||||||
|
|
||||||
### 6. Security Concerns - DONE
|
|
||||||
**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 BUFSIZ buffer (typically 8KB) may not be optimal for all scenarios, especially large files or fast storage
|
|
||||||
**Fix example:** Use adaptive buffer sizing based on file size or storage characteristics, or allow configuration via environment variable
|
|
||||||
|
|
||||||
### 8. I/O Problems
|
|
||||||
**Files affected:** `src/meta_plugin/program.rs`, `src/compression_engine/program.rs`
|
|
||||||
**Functions affected:** `MetaPluginProgram::finalize()`, `CompressionEngineProgram::open()`, `CompressionEngineProgram::create()`
|
|
||||||
**Problem example:** Meta plugin processes can block indefinitely if they hang or produce large output without proper timeouts
|
|
||||||
**Fix example:** Add timeouts to process operations and non-blocking I/O for meta plugins: `process.wait_timeout(Duration::from_secs(30))`
|
|
||||||
|
|
||||||
## 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:** Large functions doing multiple responsibilities
|
|
||||||
**Fix example:** Split into smaller functions:
|
|
||||||
- `src/modes/save.rs: mode_save()` → `setup_compression_and_plugins()`, `process_input_stream()`, `finalize_meta_plugins()`, `save_item_to_database()`
|
|
||||||
- `src/modes/diff.rs: mode_diff()` → `validate_diff_args()`, `setup_diff_pipes()`, `spawn_writer_threads()`, `execute_diff_command()`, `handle_diff_output()`
|
|
||||||
- `src/modes/diff.rs: write_item_to_pipe()` → `open_item_reader()`, `copy_item_data()`
|
|
||||||
|
|
||||||
|
|
||||||
|
|||||||
Reference in New Issue
Block a user