-
Notifications
You must be signed in to change notification settings - Fork 48
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
Proposal for v2 API #48
Comments
Consider implementing |
Updated to implement |
Can I'd be happy with this API, especially if the above can be arranged. I figure that tables for the common algorithms would just all be declared in the module, and rely on dead code elimination to not be linked in if not needed? It might make sense to seal the Write trait; that'd allow later addition of requirements. (I'm not sure even the current requirements are sufficient.) If code and RAM size matter a lot and hashing is rare, generalizing |
Having
I think that makes the most sense.
Wouldn't having |
I was rather thinking about not calculating the table at all (when RAM is a premium as well) and calculating the table's entries for each access. My point was not about implementing this now, but leaving such possibilities open by something like this: pub trait Crc<W: Width> {
fn get_init() -> W;
fn get_entry(u8) -> W;
}
pub struct TableCrc<W: Width> {
algorithm: &'static Algorithm<W>,
table: [W; 256],
}
impl<W: Width> Crc<W> for TableCrc<W> {
fn get_init() -> W {
self.algorithm.init
}
fn get_entry(i: u8) -> W {
self.table[i]
}
}
pub struct Digest<'a, W: Width, C: Crc<W>> {
crc: &'a C,
value: W,
}
impl<'a, W: Width, C: Crc<W>> Digest<'a, W, C> {
fn new(crc: &'a impl Crc<W>) -> Self {
Digest { crc, crc.get_init() }
}
} That's a bit hypothetical, though, so if the above would make things overly complicated, and at the same time doesn't allow using a CRC accelerator like the one in STM32s with the same interface (which would require going through a trait not at the Crc but at the Digest level). It's probably be better to go for a v2 as it is now rather than to fall into the second version pitfall here. |
Just throwing my two cents in but I would love if the API functioned roughly the same as https://github.com/RustCrypto/hashes |
I was going through documentation which shows to use |
With #55 merged, do we want to close out this one / track additional improvements in a separate issue? |
Published 2.0.0 after rc period |
Addresses a couple of different issues (#37, #46, #45, #40):
crc16
,crc32
, andcrc64
modulesThis represents a significant change in the API and internal structure of this library, but I believer the advantages are clear. Fully implementing these changes will take quite a bit of work. So, I wanted to get feedback before drafting a PR.
The text was updated successfully, but these errors were encountered: