cksum/hashsum: merge digest computation & various improvements#9135
cksum/hashsum: merge digest computation & various improvements#9135sylvestre merged 10 commits intouutils:mainfrom
Conversation
CodSpeed Performance ReportMerging #9135 will improve performances by ×510Comparing Summary
Benchmarks breakdown
Footnotes
|
|
GNU testsuite comparison: |
db8fb3c to
36c93eb
Compare
|
GNU testsuite comparison: |
36c93eb to
e449a04
Compare
|
GNU testsuite comparison: |
85549e7 to
cc69557
Compare
|
GNU testsuite comparison: |
sylvestre
left a comment
There was a problem hiding this comment.
Windows builds are failing
Yes, this is being discussed in #9168, I will move the PR back to draft until it's fixed |
57a9317 to
274749c
Compare
|
GNU testsuite comparison: |
274749c to
6375a7a
Compare
|
GNU testsuite comparison: |
6375a7a to
7828fb1
Compare
|
GNU testsuite comparison: |
7828fb1 to
6e191dd
Compare
|
GNU testsuite comparison: |
6e191dd to
1c9a7af
Compare
|
GNU testsuite comparison: |
1c9a7af to
6e6b545
Compare
|
GNU testsuite comparison: |
6e6b545 to
b4671a1
Compare
|
GNU testsuite comparison: |
b4671a1 to
5956f67
Compare
| (algo @ (ak::Sha2 | ak::Sha3), None) => { | ||
| Err(ChecksumError::LengthRequiredForSha(algo.to_lowercase().into()).into()) | ||
| } | ||
| // [`calculate_blake2b_length`] expects a length in bits but we |
There was a problem hiding this comment.
Comment says 'expects a length in bits' but the multiplication by 8 suggests the input is in bytes ?
There was a problem hiding this comment.
Yes, l is a length in bytes and we multiply it to give the length in bits to the function
| let mut digest = algo.create_digest(); | ||
| let (calculated_checksum, _) = | ||
| digest_reader(&mut digest, &mut file_reader, opts.binary, algo.bits).unwrap(); | ||
| digest_reader(&mut digest, &mut file_reader, false, algo.bitlen()).unwrap(); |
There was a problem hiding this comment.
maybe add a comment why it is now "false" ? :)
(maybe we should also improve the function profile not to use a bool)
There was a problem hiding this comment.
maybe we should also improve the function profile not to use a bool
I plan on doing that at some point, but I reserve that to a later patch, once #9168 is properly determined
|
|
||
| // This test is currently disabled on windows | ||
| #[test] | ||
| #[cfg_attr(windows, ignore = "Discussion is in #9168")] |
|
|
||
| let opts = ChecksumOptions { | ||
| binary, | ||
| let opts = ChecksumValidateOptions { |
There was a problem hiding this comment.
maybe we could remove it for validate_opts ?
There was a problem hiding this comment.
Not sure I understand this comment
|
GNU testsuite comparison: |
5956f67 to
876c21c
Compare
|
GNU testsuite comparison: |
876c21c to
3ce7c77
Compare
|
GNU testsuite comparison: |
3ce7c77 to
6d6a991
Compare
|
GNU testsuite comparison: |
This PR adds several improvements:
AlgoKindenum to stop relying on string comparison for checking algorithm behaviorsSizedAlgokindenum to simply represent an algorithm with a given size.