-
-
Notifications
You must be signed in to change notification settings - Fork 1.3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
cksum: implement check (Closes: #5705) #6390
Conversation
e0becb8
to
6b8ce37
Compare
GNU testsuite comparison:
|
GNU testsuite comparison:
|
should fail but getting close |
GNU testsuite comparison:
|
c053907
to
40e00b6
Compare
GNU testsuite comparison:
|
coreutils/coreutils@b5ce9fb I contributed this upstream as they were missing |
GNU testsuite comparison:
|
GNU testsuite comparison:
|
GNU testsuite comparison:
|
5c118cd
to
415d655
Compare
GNU testsuite comparison:
|
GNU testsuite comparison:
|
GNU testsuite comparison:
|
@BenWiederhake I added tests on our side (ignored) for stuff that we don't support yet (and probably quite niche) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Most of the problems I addressed are indeed fixed, and I mostly agree that it's a good idea to move most of the remainder for later. Hooray!
There are two exceptions that I think should be fixed in this PR:
- The new-ish issue
SHA4 (README.md) = 00000000
, see above - The bit-rounding-down issue, which causes us to accept dubious and obviously-wrong hashes. This should be just a simple div-mod, right?
Things that should be fixed eventually, but perhaps not right now:
- "accept files with explicit algo but algos mismatch" details
- inconsistent behavior caused by the inclusion of
b
in the algo-name regex:
$ echo 'bSD (README.md) = 0000' > foo.sums; cargo run -q cksum -c foo.sums
README.md: FAILED
cksum: WARNING: 1 computed checksum did NOT match
[$? = 1]
$ echo 'BsD (README.md) = 0000' > foo.sums; cargo run -q cksum -c foo.sums
cksum: foo.sums: no properly formatted checksum lines found
cksum: WARNING: 1 line is improperly formatted
[$? = 1]
Done :)
|
GNU testsuite comparison:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No more crashes, no more false-positives in the checking, and I couldn't find any new edge cases in the new code. Yay! This is ready to merge in my eyes.
No description provided.