Skip to content
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

Use new *Dirty traits from the digest crate #153

Merged
merged 1 commit into from
Jun 9, 2020
Merged

Conversation

tarcieri
Copy link
Member

@tarcieri tarcieri commented Jun 9, 2020

Updates all of the hash implementations in this repo to use the new *Dirty traits which support blanket impls of the original traits, which either consume the hasher instance or can be reset.

This will also provide a marginal efficiency boost, at least until placement return lands (which, as it were, may be soon).

@tarcieri tarcieri requested a review from newpavlov June 9, 2020 21:52
Updates all of the hash implementations in this repo to use the new
`*Dirty` traits which support blanket impls of the original traits,
which either consume the hasher instance or can be reset.

This will also provide a marginal efficiency boost, at least until
placement return lands (which, as it were, may be soon).
fn finalize_fixed(mut self) -> GenericArray<u8, U32> {
{
let self_state = &mut self.state;
fn finalize_into_dirty(&mut self, out: &mut GenericArray<u8, U32>) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it could be worth to be consistent and use digest::Output<Self> across all crates.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The reason I couldn't here is because it doesn't impl Default, and digest::Output (presently, at least) uses the Digest trait:

https://github.com/RustCrypto/traits/blob/master/digest/src/digest.rs#L96

It could potentially use FixedOuput instead.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hrmm, changing it to use FixedOutput is not trivial... it means you can't use it with Digest because not all Digests necessarily impl FixedOutput, and that kind of defeats the point.

I think we should leave it as-is. That said, I think we can fix the version used in the macro.

@newpavlov
Copy link
Member

newpavlov commented Jun 9, 2020

Dirty traits fit even better than I expected.

@tarcieri
Copy link
Member Author

tarcieri commented Jun 9, 2020

Indeed, surprisingly easy refactor for what it is.

I'll go ahead and merge and circle back on changing digest::Output to use FixedOutput.

@tarcieri tarcieri merged commit 26bea13 into master Jun 9, 2020
@tarcieri tarcieri deleted the dirty-digest-crate branch June 9, 2020 23:22
tarcieri added a commit that referenced this pull request Jun 10, 2020
tarcieri added a commit that referenced this pull request Jun 10, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants