Add custom parameter support #1
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Update stub file with new CrcFast\Params class and function signatures - Add CrcFast\Params class definition with constructor and getter methods - Update existing function signatures to accept int|CrcFast\Params for algorithm parameter
- Run PHP's gen_stub.php to regenerate crc_fast_arginfo.h from the updated stub file - Verify that new function signatures and class definitions are properly generated
- [x] 3. Implement CrcFast\Params class in C extension
- [x] 3.1 Define php_crc_fast_params_obj structure and object handlers
- Create structure containing CrcFastParams and zend_object
- Implement create_object, free_obj, and other standard object handlers
- [x] 3.2 Implement CrcFast\Params constructor
- Parse constructor parameters (width, poly, init, refin, refout, xorout, check, keys)
- Validate parameter values according to design constraints
- Create and populate CrcFastParams struct
- Handle optional keys parameter (generate if null, validate if provided)
- [x] 3.3 Implement CrcFast\Params getter methods
- Implement getWidth(), getPoly(), getInit(), getRefin(), getRefout(), getXorout(), getCheck()
- Implement getKeys() method that always returns the computed keys array
- [x] 4. Update existing functions to support custom parameters
- [x] 4.1 Create parameter detection helper function
- Add helper to determine if parameter is int (algorithm constant) or CrcFast\Params object
- Extract CrcFastParams struct from Params object when needed
- _Requirements: 3.1, 3.2, 3.3_
- [x] 4.2 Update CrcFast\hash() function
- Modify function to accept int|CrcFast\Params for algorithm parameter
- Use crc_fast_checksum() for predefined algorithms
- Use crc_fast_checksum_with_params() for custom parameters
- Maintain existing behavior for all predefined algorithms
- _Requirements: 3.1, 4.1, 4.2_
- [x] 4.3 Update CrcFast\hash_file() function
- Modify function to accept int|CrcFast\Params for algorithm parameter
- Use crc_fast_checksum_file() for predefined algorithms
- Use crc_fast_checksum_file_with_params() for custom parameters
- Maintain existing behavior for all predefined algorithms
- _Requirements: 3.2, 4.1, 4.2_
- [x] 4.4 Update CrcFast\combine() function
- Modify function to accept int|CrcFast\Params for algorithm parameter
- Use crc_fast_checksum_combine() for predefined algorithms
- Use crc_fast_checksum_combine_with_custom_params() for custom parameters
- Maintain existing checksum parsing logic for both binary and hex inputs
- _Requirements: 3.3, 4.1, 4.2_
The XtOffsetOf macro was causing some issues with obj->algorithm values.
- [x] 5. Update CrcFast\Digest class to support custom parameters
- [x] 5.1 Modify Digest constructor
- Update constructor to accept int|CrcFast\Params for algorithm parameter
- Use crc_fast_digest_new() for predefined algorithms
- Use crc_fast_digest_new_with_params() for custom parameters
- Store parameter type information for later use in formatting
- _Requirements: 3.4, 4.1, 4.2_
- [x] 5.2 Update result formatting for custom parameters
- Modify php_crc_fast_format_result() to handle custom parameters
- Determine output width (32 or 64 bit) from custom parameters
- Ensure binary and hex output formats work correctly for custom algorithms
- _Requirements: 1.3, 4.3_
- [x] 6. Add comprehensive error handling - Add parameter validation with descriptive error messages - Handle C library errors gracefully with appropriate PHP exceptions - Validate keys array length and values when provided - _Requirements: 5.1, 5.2, 5.3, 5.4_
- [x] 7. Create unit tests for CrcFast\Params class - Test constructor with valid parameters - Test constructor with invalid parameters (wrong width, out-of-range values) - Test getter methods return correct values - Test keys parameter handling (both provided and auto-generated) - _Requirements: 1.1, 1.2, 5.1, 5.2_
Adds notes about test debugging
- [x] 8.1 Test CrcFast\hash() with custom parameters
- Create custom parameters that match existing predefined algorithms
- Verify identical results between custom and predefined algorithms
- Test both 32-bit and 64-bit custom algorithms
- _Requirements: 3.1, 4.1_
- [x] 8.2 Test CrcFast\hash_file() with custom parameters
- Test file hashing with custom parameters
- Verify results match hash() function for same data
- Test both binary and hex output formats
- _Requirements: 3.2, 4.1_
- [x] 8.3 Test CrcFast\combine() with custom parameters
- Test checksum combination with custom parameters
- Verify results match manual calculation
- Test with both binary and hex checksum inputs
- _Requirements: 3.3, 4.1_
- [x] 8.4 Test CrcFast\Digest class with custom parameters
- Test digest creation, update, and finalization with custom parameters
- Test digest combination with custom parameters
- Verify results match direct hash() function calls
- _Requirements: 3.4, 4.1_
- [x] 9. Create backward compatibility tests - Run all existing tests to ensure no regressions - Verify all existing function signatures still work - Verify all existing constants and behavior unchanged - _Requirements: 4.1, 4.2, 4.3_
- [x] 10. Build and test extension - Compile extension with updated code - Run complete test suite to verify all functionality - Test extension loading and basic functionality - _Requirements: All requirements verification_
- [x] 11. Fix build warnings for zend_long format specifiers - Fix all format string warnings by using proper format specifiers for zend_long type - Change %ld to %lld and %lx to %llx for zend_long variables - Rebuild and verify all tests still pass with clean compilation - _Requirements: Code quality and maintainability_
Contributor
There was a problem hiding this comment.
Pull Request Overview
This pull request adds support for custom CRC parameters to the PHP CRC Fast extension, allowing developers to define their own CRC algorithms beyond the predefined set. The implementation introduces a new CrcFast\Params class and extends existing functions (hash(), hash_file(), combine(), and the Digest class) to accept either integer algorithm constants or custom parameter objects, while maintaining full backward compatibility.
Key changes:
- New
CrcFast\Paramsclass with constructor and getter methods for defining custom CRC algorithms - Extended function signatures to accept
int|CrcFast\Paramsfor algorithm parameters - Comprehensive test coverage including unit tests, integration tests, error handling tests, and backward compatibility tests
Reviewed Changes
Copilot reviewed 22 out of 22 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| php_crc_fast.h | Adds php_crc_fast_params_obj struct and handlers; defines container_of macro for object conversion |
| php_crc_fast.cpp | Implements CrcFast\Params class methods, extends existing functions to support custom parameters, adds parameter validation and error handling |
| crc_fast.stub.php | Adds Params class definition and updates function signatures to accept int|Params |
| crc_fast_arginfo.h | Auto-generated from stub file with new class and function signatures |
| tests/params_*.phpt | Unit tests for Params class constructor, getters, and validation |
| tests/*_custom_params.phpt | Integration tests for custom parameters with hash(), hash_file(), combine(), and Digest |
| tests/error_handling_*.phpt | Error handling tests for all components |
| tests/backward_compatibility.phpt | Ensures existing functionality remains unchanged |
| tests/combine.phpt | Modified to add finalize call before combine operation |
| .kiro/steering/essential-commands.md | Documentation for build and test commands |
| .kiro/specs/custom-crc-parameters/* | Requirements, design, and implementation task documentation |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The Problem
The crc-fast library has added support for custom parameters, but this extension doesn't support it.
The Solution
Add support.
Planned version bump
MINORLinks