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

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

Open
james-rms opened this issue Sep 6, 2022 · 6 comments
Open

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

james-rms opened this issue Sep 6, 2022 · 6 comments

Comments

@james-rms
Copy link

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?

@james-rms james-rms changed the title Docs for keeping a Digest in a struct w/ 3.0 API recommended method for keeping a Digest in a struct w/ 3.0 API Sep 6, 2022
@akhilles
Copy link
Collaborator

akhilles commented Sep 7, 2022

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
Copy link
Author

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
Copy link
Collaborator

akhilles commented Sep 8, 2022

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
Copy link

plwalsh commented Sep 15, 2022

@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
Copy link
Collaborator

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
Copy link

plwalsh commented Sep 15, 2022

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

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

No branches or pull requests

3 participants