mrhooray / crc-rs

Rust implementation of CRC(16, 32, 64) with support of various standards
Apache License 2.0
187 stars 49 forks source link

recommended method for keeping a `Digest` in a struct w/ 3.0 API #73

Open james-rms opened 2 years ago

james-rms commented 2 years ago

Hello,

I'm trying to make a wrapper for a std::io::Write object that keeps a running CRC. This looks something like this:

struct Wrapper<W> {
   w: W,
   crc: Crc<u32>,
}

impl <W: std::io::Write> std::io::Write for Wrapper {
    fn write(&mut self, buf: &[u8]) -> std::io::Result<usize> {
        self.crc.digest().update(buf);
        self.w.write(buf)
    }
}

However this is wrong, because the running state of the digest gets discarded on every write call (i think). I feel like what I want is crc.update(), but it's not pub. I could also figure out a way to store both the crc and the digest in the struct, but that would involve internal references, which are a pain to work with.

Is there some recommended way to do this? Is this a good case for making Crc::update pub?

akhilles commented 2 years ago

Can Crc be initialized at compile-time?

pub const CASTAGNOLI: Crc<u32> = Crc::<u32>::new(&CRC_32_ISCSI);

Then, Digest will be Digest<'static, u32> and you won't have to deal with internal references.

Crc::new is quite slow because it generates the tables, so you typically don't want to do it at runtime.

james-rms commented 2 years ago

Oh nice, I didn't think of that at all. I'm happy to make a PR to put an example of that in the docs somewhere.

akhilles commented 2 years ago

Oh nice, I didn't think of that at all. I'm happy to make a PR to put an example of that in the docs somewhere.

Sure, that'd be great.

plwalsh commented 1 year ago

@akhilles The Rust docs state that "constants are essentially inlined." Does that mean that by setting Crc::new() to a const, it will be evaluated (i.e. table generated) every time the constant is accessed/referenced? Or will it still only be evaluated once? And does it make any difference if the const is defined as an "associated constant" inside an impl, as opposed to being at the module level?

akhilles commented 1 year ago

It will only be evaluated at compile-time and it doesn't matter if it's an associated const. The output of Crc::new() is what gets inlined (in some cases).

So, if Crc::new() is assigned to a const, table generation will not occur at runtime. Not even once.

plwalsh commented 1 year ago

Makes sense, thanks! Just wasn't sure what happened under the hood.