search

mrhooray / crc-rs

Rust implementation of CRC(16, 32, 64) with support of various standards
Apache License 2.0
188 stars 49 forks source link

Add Crc::digest_with_initial #69

Closed voidc closed 2 years ago

voidc commented 2 years ago

It may sometimes be useful to compute CRC values with an initial value different from the one specified by the used algorithm. In this case, users currently have to create a new Algorithm for each initial value. This is particularly problematic if the initial values are not statically known, since Crc::new takes a static reference to an algorithm (see also #68). With the small change proposed in this PR, an initial value may optionally be supplied when creating a Digest.

voidc commented 2 years ago

For reference, the crc32fast crate provides a new_with_initial function.

akhilles commented 2 years ago

Would it be possible to add some test cases with the expected checksums generated by another crc impl? I'm not sure if the way we handle reflection of custom init is considered "correct".

voidc commented 2 years ago

I'd say the way it is handled now is conceptually correct since the custom init value is treated in the same way as Algorithm::init which is IMO what users would expect to happen. If you want, I can add a comment to the digest_with_initial methods documenting this behavior. As for tests, I'm not sure what would be a good alternative impl for comparison.

akhilles commented 2 years ago

I'd say the way it is handled now is conceptually correct since the custom init value is treated in the same way as Algorithm::init which is IMO what users would expect to happen. If you want, I can add a comment to the digest_with_initial methods documenting this behavior. As for tests, I'm not sure what would be a good alternative impl for comparison.

A comment would be enough IMO, and then we can merge.

voidc commented 2 years ago

Thanks for merging! Would it be possible to release a new version including this change on crates.io?

akhilles commented 2 years ago

I'll try to cut a minor release (v2.2.0) by tomorrow. There are some breaking changes that I either need to patch or exclude.