Skip to content

Commit dfc6de6

Browse files
onethumbonethumb
committed
Create spec for future-proofing CrcParams
1 parent bf7d24a commit dfc6de6

File tree

3 files changed

+378
-0
lines changed

3 files changed

+378
-0
lines changed

‎.kiro/specs/future-proof-crc-params/design.md

Lines changed: 216 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,216 @@
1+
# Design Document
2+
3+
## Overview
4+
5+
This design implements a future-proof CrcParams structure using an internal enum-based key storage system that can expand to support different key array sizes without breaking compatibility. The approach maintains the simplicity of const definitions while providing safe key access and zero runtime overhead through compiler optimizations.
6+
7+
## Architecture
8+
9+
### Core Components
10+
11+
1. **CrcKeysStorage Enum**: Internal storage that can hold different key array sizes
12+
2. **CrcParams Structure**: Updated to use CrcKeysStorage internally while maintaining public API
13+
3. **Safe Accessor Methods**: Bounds-checked key access methods on CrcParams
14+
4. **Helper Functions**: Const-friendly constructors for CrcKeysStorage variants
15+
16+
### Design Principles
17+
18+
- **Zero Runtime Overhead**: Enum dispatch optimized away by compiler
19+
- **Backwards Compatibility**: Existing const definitions require minimal changes
20+
- **Gradual Migration**: Can be implemented in phases without breaking builds
21+
- **Safety First**: Bounds checking prevents panics from out-of-range access
22+
23+
## Components and Interfaces
24+
25+
### CrcKeysStorage Enum
26+
27+
```rust
28+
#[derive(Clone, Copy, Debug)]
29+
enum CrcKeysStorage {
30+
/// Current 23-key format (for existing algorithms which includes 256 byte folding distances)
31+
KeysFold256([u64; 23]),
32+
/// Future 25-key format (for potential future expanded folding distances, for testing purposes only)
33+
KeysFutureTest([u64; 25]),
34+
// Additional variants can be added as needed
35+
}
36+
```
37+
38+
**Key Methods:**
39+
- `get_key(index: usize) -> u64`: Safe key access with bounds checking
40+
- `key_count() -> usize`: Returns actual number of keys available
41+
- `from_keys_fold_256(keys: [u64; 23]) -> Self`: Const constructor for 23-key arrays
42+
- `from_keys_fold_future_test(keys: [u64; 25]) -> Self`: Const constructor for 25-key arrays
43+
44+
### Updated CrcParams Structure
45+
46+
```rust
47+
#[derive(Clone, Copy, Debug)]
48+
pub struct CrcParams {
49+
pub algorithm: CrcAlgorithm,
50+
pub name: &'static str,
51+
pub width: u8,
52+
pub poly: u64,
53+
pub init: u64,
54+
pub refin: bool,
55+
pub refout: bool,
56+
pub xorout: u64,
57+
pub check: u64,
58+
pub keys: CrcKeysStorage, // Changed from [u64; 23]
59+
}
60+
```
61+
62+
**Key Methods:**
63+
- `get_key(index: usize) -> u64`: Delegates to CrcKeysStorage
64+
- `get_key_checked(index: usize) -> Option<u64>`: Optional key access
65+
- `key_count() -> usize`: Returns actual key count
66+
67+
### Const Definition Pattern
68+
69+
```rust
70+
// Before (Phase 2):
71+
pub const CRC32_ISCSI: CrcParams = CrcParams {
72+
// ... other fields unchanged ...
73+
keys: KEYS_1EDC6F41_REFLECTED, // [u64; 23]
74+
};
75+
76+
// After (Phase 3):
77+
pub const CRC32_ISCSI: CrcParams = CrcParams {
78+
// ... other fields unchanged ...
79+
keys: CrcKeysStorage::from_keys_fold_256(KEYS_1EDC6F41_REFLECTED),
80+
};
81+
```
82+
83+
## Data Models
84+
85+
### Key Storage Variants
86+
87+
| Variant | Array Size | Use Case |
88+
|---------|------------|----------|
89+
| KeysFold256 | [u64; 23] | Current implementation (128/256-byte folding) |
90+
| KeysFutureTest | [u64; 25] | Future expansion |
91+
92+
### Migration States
93+
94+
| Phase | CrcParams.keys Type | Architecture Code | Const Definitions |
95+
|-------|-------------------|------------------|------------------|
96+
| 1 | [u64; 23] | Direct access | Unchanged |
97+
| 2 | [u64; 23] | Safe accessors | Unchanged |
98+
| 3 | CrcKeysStorage | Safe accessors | Updated |
99+
100+
## Error Handling
101+
102+
### Bounds Checking Strategy
103+
104+
1. **Safe Default**: Out-of-bounds key access returns 0 instead of panicking
105+
2. **Optional Access**: `get_key_checked()` returns `None` for invalid indices
106+
3. **Graceful Degradation**: Code continues to function with missing keys
107+
108+
### Error Scenarios
109+
110+
| Scenario | Behavior | Rationale |
111+
|----------|----------|-----------|
112+
| Access key[30] with 23-key storage | Returns 0 | Allows future expansion without breaking existing code |
113+
| Invalid key index | Returns 0 | Prevents panics, maintains stability |
114+
| Empty key storage | Returns 0 for all indices | Defensive programming |
115+
116+
## Testing Strategy
117+
118+
### Unit Tests
119+
120+
1. **CrcKeysStorage Tests**:
121+
- Verify correct key storage and retrieval for each variant
122+
- Test bounds checking behavior
123+
- Validate const constructor functions
124+
125+
2. **CrcParams Integration Tests**:
126+
- Verify safe accessor methods work correctly
127+
- Test backwards compatibility with existing const definitions
128+
- Validate zero runtime overhead through benchmarks
129+
130+
3. **Migration Tests**:
131+
- Test each phase independently
132+
- Verify existing functionality remains intact
133+
- Validate const definition updates
134+
135+
### Compatibility Tests
136+
137+
1. **Third-Party Simulation**:
138+
- Create mock third-party const definitions
139+
- Verify they continue working through all phases
140+
- Test key access patterns used by external code
141+
142+
2. **Performance Tests**:
143+
- Benchmark key access performance vs direct array access
144+
- Verify compiler optimizations eliminate enum dispatch
145+
- Measure memory usage impact
146+
147+
### Integration Tests
148+
149+
1. **Architecture Code Tests**:
150+
- Update existing architecture tests to use safe accessors
151+
- Verify SIMD operations work correctly with new key access
152+
- Test folding operations across different key storage variants
153+
154+
2. **End-to-End Tests**:
155+
- Verify CRC calculations remain correct after migration
156+
- Test custom CrcParams creation and usage
157+
- Validate `get-custom-params` binary output
158+
159+
## Implementation Phases
160+
161+
### Phase 1: Add New Types
162+
- Add CrcKeysStorage enum to codebase
163+
- Add helper methods to CrcParams (delegating to existing keys field)
164+
- Maintain existing [u64; 23] field for compatibility
165+
- All tests continue to pass
166+
167+
### Phase 2: Update Architecture Code
168+
- Replace direct key array access with safe accessor methods
169+
- Update SIMD and folding code to use `params.get_key(index)`
170+
- Maintain backwards compatibility
171+
- Performance remains identical
172+
173+
### Phase 3: Switch to New Storage
174+
- Change CrcParams.keys field from [u64; 23] to CrcKeysStorage
175+
- Update all const definitions to use CrcKeysStorage::from_keys_23()
176+
- Update `get-custom-params` binary output format
177+
- This is the only breaking change, but minimal impact
178+
179+
## Performance Considerations
180+
181+
### Compiler Optimizations
182+
183+
The Rust compiler optimizes enum dispatch when:
184+
1. All variants have the same access pattern
185+
2. The enum is used in hot paths with predictable patterns
186+
3. Inlining is enabled for accessor methods
187+
188+
Expected assembly output for `params.get_key(21)`:
189+
```assembly
190+
; Same as direct array access keys[21]
191+
mov rax, qword ptr [rdi + 168 + 21*8]
192+
```
193+
194+
### Memory Layout
195+
196+
| Storage Type | Memory Usage | Alignment |
197+
|--------------|--------------|-----------|
198+
| KeysFold256 | 184 bytes | 8-byte aligned |
199+
| KeysFutureTest | 200 bytes | 8-byte aligned |
200+
| Enum overhead | 0 bytes | (optimized away) |
201+
202+
## Security Considerations
203+
204+
### Bounds Safety
205+
206+
The new design eliminates array bounds panics, which could be exploited in unsafe contexts. Safe key access prevents:
207+
- Buffer overflow attacks through malicious key indices
208+
- Denial of service through panic-induced crashes
209+
- Information disclosure through out-of-bounds memory access
210+
211+
### Const Safety
212+
213+
All const definitions remain compile-time validated, preventing:
214+
- Runtime key generation vulnerabilities
215+
- Dynamic key modification attacks
216+
- Timing-based side-channel attacks on key access

‎.kiro/specs/future-proof-crc-params/requirements.md

Lines changed: 66 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,66 @@
1+
# Requirements Document
2+
3+
## Introduction
4+
5+
This feature implements a future-proof CrcParams structure that can expand to support additional folding keys (for example, larger folding distances) without breaking API compatibility for third-party applications. The solution maintains the simplicity of const definitions while providing safe key access and internal flexibility for future expansion.
6+
7+
## Requirements
8+
9+
### Requirement 1
10+
11+
**User Story:** As a library maintainer, I want to expand CRC folding key support from 23 to 24+ keys for potentially larger folding distances in the future, so that I can improve performance for large data processing.
12+
13+
#### Acceptance Criteria
14+
15+
1. WHEN I add support for larger folding distances THEN existing third-party applications with hardcoded 23-key CrcParams SHALL continue to compile and function correctly
16+
2. WHEN I expand key arrays from 23 to 24+ elements THEN existing const definitions SHALL require minimal changes (only the keys field)
17+
18+
### Requirement 2
19+
20+
**User Story:** As a third-party application developer, I want to define custom CrcParams as const definitions, so that I can embed CRC configurations directly in my code without runtime overhead.
21+
22+
#### Acceptance Criteria
23+
24+
1. WHEN I define a custom CrcParams const THEN I SHALL be able to use the same simple struct literal syntax as currently exists
25+
2. WHEN I access CRC keys through the CrcParams interface THEN the performance SHALL be identical to direct array access (zero runtime overhead)
26+
3. WHEN the library expands key support THEN my existing const definitions SHALL continue to work without modification
27+
28+
### Requirement 3
29+
30+
**User Story:** As a library maintainer, I want safe key access methods that prevent array bounds panics, so that the library is robust against future expansion and misuse.
31+
32+
#### Acceptance Criteria
33+
34+
1. WHEN architecture code accesses CRC keys THEN it SHALL use bounds-checked methods instead of direct array indexing
35+
2. WHEN code requests key count information THEN it SHALL receive the actual number of available keys for that CrcParams instance
36+
37+
### Requirement 4
38+
39+
**User Story:** As a library maintainer, I want internal flexibility to support different key array sizes, so that I can optimize different CRC algorithms with varying folding distance requirements.
40+
41+
#### Acceptance Criteria
42+
43+
1. WHEN I create CrcParams with 23 keys THEN the system SHALL store and access exactly 23 keys efficiently
44+
2. WHEN I create CrcParams with 25 keys THEN the system SHALL store and access exactly 25 keys efficiently
45+
4. WHEN the compiler optimizes the code THEN enum dispatch for key access SHALL be eliminated (zero runtime overhead)
46+
47+
### Requirement 5
48+
49+
**User Story:** As a library maintainer, I want to migrate existing code gradually, so that I can implement the changes in phases without breaking the build at any point.
50+
51+
#### Acceptance Criteria
52+
53+
1. WHEN I add the new CrcKeysStorage types THEN existing code SHALL continue to compile and function
54+
2. WHEN I update architecture code to use safe accessors THEN the change SHALL be backward compatible
55+
3. WHEN I switch CrcParams to use CrcKeysStorage THEN the migration SHALL require only updating const definitions
56+
4. WHEN each phase is complete THEN all existing tests SHALL continue to pass
57+
58+
### Requirement 6
59+
60+
**User Story:** As a third-party application developer, I want the `get-custom-params` binary to output the updated `CrcParams` const definition using the new key storage approach, so that I can easily generate future-proof custom CRC parameter definitions.
61+
62+
#### Acceptance Criteria
63+
64+
1. WHEN I run the `get-custom-params` binary THEN it SHALL output CrcParams const definitions using CrcKeysStorage::from_keys_23()
65+
2. WHEN I copy the generated const definition THEN it SHALL compile and work correctly with the new CrcParams structure
66+
3. WHEN the output format changes THEN the generated code SHALL remain compatible with the current CrcParams API

‎.kiro/specs/future-proof-crc-params/tasks.md

Lines changed: 96 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,96 @@
1+
# Implementation Plan
2+
3+
- [ ] 1. Phase 1: Add CrcKeysStorage enum and helper methods
4+
- Add CrcKeysStorage enum with KeysFold256 and KeysFutureTest variants
5+
- Implement get_key() and key_count() methods on CrcKeysStorage
6+
- Add const constructor methods from_keys_fold_256() and from_keys_fold_future_test()
7+
- Add safe accessor methods to CrcParams that delegate to existing keys field
8+
- Write comprehensive unit tests for CrcKeysStorage functionality
9+
- _Requirements: 4.1, 4.2, 4.4, 5.1_
10+
11+
- [ ] 2. Phase 2: Update architecture code to use safe accessors
12+
- [ ] 2.1 Update SIMD folding code in src/arch/ to use params.get_key()
13+
- Replace direct keys[index] access with params.get_key(index) in algorithm.rs
14+
- Update VPCLMULQDQ code to use safe key access methods
15+
- Update aarch64 and x86 architecture-specific code
16+
- _Requirements: 3.1, 5.2_
17+
18+
- [ ] 2.2 Update CRC32 algorithm code to use safe accessors
19+
- Modify src/crc32/algorithm.rs to use params.get_key() instead of keys[index]
20+
- Update fusion code in src/crc32/fusion/ if it accesses keys directly
21+
- _Requirements: 3.1, 5.2_
22+
23+
- [ ] 2.3 Update CRC64 algorithm code to use safe accessors
24+
- Modify src/crc64/algorithm.rs to use params.get_key() instead of keys[index]
25+
- Update any other CRC64-specific code that accesses keys directly
26+
- _Requirements: 3.1, 5.2_
27+
28+
- [ ] 2.4 Run performance benchmarks to verify zero overhead
29+
- Benchmark key access performance before and after changes
30+
- Verify compiler optimizations eliminate any performance regression
31+
- Document that performance remains identical to direct array access
32+
- _Requirements: 2.2, 4.4_
33+
34+
- [ ] 3. Phase 3: Switch CrcParams to use CrcKeysStorage
35+
- [ ] 3.1 Update CrcParams struct definition
36+
- Change keys field from [u64; 23] to CrcKeysStorage
37+
- Update CrcParams accessor methods to delegate to CrcKeysStorage
38+
- Remove temporary delegation methods added in Phase 1
39+
- _Requirements: 5.3_
40+
41+
- [ ] 3.2 Update all CRC32 const definitions
42+
- Update src/crc32/consts.rs to use CrcKeysStorage::from_keys_fold_256()
43+
- Modify all CRC32_* const definitions to use new key storage format
44+
- Ensure all existing key arrays are properly wrapped
45+
- _Requirements: 1.2, 2.1_
46+
47+
- [ ] 3.3 Update all CRC64 const definitions
48+
- Update src/crc64/consts.rs to use CrcKeysStorage::from_keys_fold_256()
49+
- Modify all CRC64_* const definitions to use new key storage format
50+
- Ensure all existing key arrays are properly wrapped
51+
- _Requirements: 1.2, 2.1_
52+
53+
- [ ] 3.4 Update get-custom-params binary output
54+
- Modify src/bin/get-custom-params.rs to output CrcKeysStorage format
55+
- Update output template to use CrcKeysStorage::from_keys_fold_256()
56+
- Test that generated const definitions compile and work correctly
57+
- _Requirements: 6.1, 6.2, 6.3_
58+
59+
- [ ] 3.5 Update cache system for new CrcParams structure
60+
- Modify src/cache.rs to work with CrcKeysStorage-based CrcParams
61+
- Update CrcParams::new() method to use new key storage format
62+
- Ensure cache functionality remains intact after structural changes
63+
- _Requirements: 2.3, 5.3_
64+
65+
- [ ] 4. Create comprehensive test suite for future-proof functionality
66+
- [ ] 4.1 Add unit tests for bounds checking behavior
67+
- Test that get_key() returns 0 for out-of-bounds indices
68+
- Test that get_key_checked() returns None for invalid indices
69+
- Verify key_count() returns correct values for different storage variants
70+
- _Requirements: 3.2_
71+
72+
- [ ] 4.2 Add integration tests for third-party compatibility
73+
- Create mock third-party const definitions using new format
74+
- Test that existing key access patterns continue to work
75+
- Verify backwards compatibility throughout migration phases
76+
- _Requirements: 1.1, 2.3_
77+
78+
- [ ] 4.3 Add performance regression tests
79+
- Benchmark CRC calculation performance before and after changes
80+
- Verify that key access performance matches direct array access
81+
- Test memory usage impact of enum-based storage
82+
- _Requirements: 2.2, 4.4_
83+
84+
- [ ] 4.4 Add future expansion simulation tests
85+
- Create test CrcParams using KeysFutureTest variant with 25 keys
86+
- Test that code gracefully handles different key array sizes
87+
- Verify that expansion to larger key arrays works as designed
88+
- _Requirements: 1.1, 4.2_
89+
90+
- [ ] 5. Validate migration and run full test suite
91+
- Run cargo test to ensure all existing tests pass
92+
- Run cargo clippy to ensure code quality standards
93+
- Run cargo fmt to ensure consistent formatting
94+
- Verify that all CRC calculations produce identical results
95+
- Test that third-party usage patterns remain functional
96+
- _Requirements: 5.4_

0 commit comments

Comments
 (0)