cksum/hashsum: merge digest computation & various improvements#9135
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 (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
| #[test] | ||
| #[cfg_attr(windows, ignore = "Discussion is in #9168")] |
| @@ -247,52 +197,52 @@ pub fn uumain(mut args: impl uucore::Args) -> UResult<()> { | |||
|
|
|||
| let verbose = ChecksumVerbose::new(status, quiet, warn); | |||
|
|
|||
| 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.