mrhooray / crc-rs Goto Github PK

lazy_static requires a lot of things that are not always available on embedded systems.

Problem

I'm installing my project with --locked. cargo warns package crc-catalog v2.3.0 in Cargo.lock is yanked

$ cargo install --locked super_speedy_syslog_searcher
    Updating crates.io index
  Downloaded super_speedy_syslog_searcher v0.6.69
  Downloaded 1 crate (461.5 KB) in 0.00s
  Installing super_speedy_syslog_searcher v0.6.69
warning: package `crc-catalog v2.3.0` in Cargo.lock is yanked in registry `crates-io`, consider running without --locked
  ...

Reviewing cargo tree

├── lzma-rs v0.3.0
│   ├── byteorder v1.5.0
│   └── crc v3.0.1
│       └── crc-catalog v2.3.0

Solution

crate crc-catalog is, as of this time, at version v2.4.0. Can this crc-rs project update the dependency so there are no warning when using --locked ?

(I also need to submit an Issue or PR to lzma-rs which I'll when this Issue is fixed)

I can submit a PR if you prefer.

I'm trying to update the crc dependency for my crate, and I need the core::hash::Hasher trait. Related to #62, I'm having some issues I can't figure out.

Code: https://github.com/malwaredb/lzjd-rs/blob/ee6183a692b8691d60550ef51f22132243f0b263/src/crc32.rs#L21

The closest I've come is this weird code which doesn't work:

pub struct CRC32Hasher<'a> {
    crc: crc::Crc<u32>,
    digest: Option<&'a crc::Digest<'a, u32>>,
}

impl<'a> CRC32Hasher<'a> {
    fn new() -> Self {
        let crc = crc::Crc::<u32>::new(&CRC_32_BZIP2);
        Self { crc, digest: None }
    }
}

impl<'a> Hasher for CRC32Hasher<'a> {
    fn finish(&self) -> u64 {
        self.digest.clone().unwrap().finalize() as u64
    }

    fn write(&mut self, bytes: &[u8]) {
        if self.digest.is_none() {
            self.digest = Some(self.crc.digest()); // doesn't live long enough
        }

        //self.digest.as_ref().unwrap().update(bytes);
    }
}

/// std::hash::BuildHasher that builds CRC32Hashers
#[derive(Clone, Default)]
pub struct CRC32BuildHasher<'a> {
    phantom: Option<&'a [u8]>, // need lifetimes specified, and need a member to connect a lifetime, but this data isn't used
}

impl<'a> BuildHasher for CRC32BuildHasher<'a> {
    type Hasher = CRC32Hasher<'a>;

    fn build_hasher(&self) -> Self::Hasher {
        CRC32Hasher::new()
    }
}

1. The finalize in the crc crate takes ownership, which causes a problem with the Hasher train.
2. I can't have crc::Crc::<u32>::new(&CRC_32_BZIP2)::digest(); as the sole member of the struct of a complaint that the intermediate value doesn't last long enough. Nor does have the variable in the function scope work, since the borrow checker also complains about the reference to the local variable.

But having the Digest struct in this crate already implement the Hasher trait would be helpful, and probably useful to others as well. But maybe I'm missing something?

Blocked by rust-lang/rust#86656.

I have noticed that the current implementation still uses table lookup method. Perhaps introducing acceleration instructions for different platforms could significantly improve the computational speed. I saw in #83 that it is currently waiting for the completion of the std::simd feature before implementing this. Is that correct?

Because I would like to implement Intel's SSE4.2 acceleration for CRC calculations in crc-rs, but I'm unsure if this aligns with crc-rs's future development plans.

Wikipedia suggests that creating the tables can be done quicker by only computing for the powers of two, and then generating every other value by xoring the powers of two.

Whether or not this makes a big enough difference to matter is another question entirely, though.

Before crc-3.2.0, Digest<'a, W> was Clone if W: Clone + Width. In crc-3.2.0, Digest<'a, W> = Digest<'a, W, Table<1>>, which is not Clone because Table<const L: usize> is never Clone. This is an API-breaking change.

I'd like to replace the following code implementing the 'mpeg2' crc32 with usage the crc-rs crate,

https://github.com/dholroyd/mpeg2ts-reader/blob/master/src/mpegts_crc.rs

Just plugging in the polynomial (0x04C11DB7 I think) didn't seem to work. Are there some extra pre/post processing steps required? I believe I tried inverting the result, but that didn't seem to be correct either.

Thanks!

Just wanted to throw this out there since I hadn't heard of them before. cargo workspaces could probably be used for the sub-crate in this crate. The sub-crate still needs to be published, but it seems like that will happen automatically (and maybe use the root crates version?). I haven't looked into it too extensively, but thought you might want to know.

I'm writing a RAM stub in assembly for an embedded system that I'm going to use to quickly CRC regions of flash memory as part of the process of flashing an MCU. I was going to use this crate to generate CRC lookup tables that I would also load via the debug probe at a known address to accelerate this, but the table-access functionality is private — I just need the whole table that the crate already generates.

I can work around this -- I've just copy-pasted the crc8 and crc8_table functions into my crate, but for my use-case it'd be a nice, useful addition to this crate's API.

Would you accept a PR to expose the generated tables and table generation functions? I'd like to:

• Either make the table field of Crc public or add a pub const fn Crc::table(&self) -> I::Table
• Make the table module and its functions public

Writing components that maintain generic CRCs is difficult using this crate because the Width trait is not used to its full extent. While the generic component can demand that a W: Width be specified, many places still require a concrete type because a W: Width is insufficient.

The two places I ran into this was the lack of an .update() on a digest (it's only implemented for the 5 concrete widths) and the .digest() method.

I think that it'd be possible to move to generic implementations and leverage the Width trait; code that currently sits in the concrete implementations would move into Width. The change would almost be compatible: As Width is not sealed, the trait would need to gain non-provided methods. (It's a change that's very unlikely to break any code, as just implementing Width doesn't let a user do anything with it so far, but it's technically still breaking).

As a side effect, the built documentation would become much more usable -- for example, the crc::Crc struct would only have one impl block instead of 5.

Please can you add your license text to the repository.

I think the intention is for MIT, as indicated by the README and the Cargo.toml, however, as MIT has copyright and license statement provisions, the text is required in order for users/distributors to comply with the license.

Thank you!

Commit d171015 in v1.9.0 adds a 4th CalcType arg to the crc::crcXX::update() routine.

This breaks all current users of update() in transitioning between 1.8.1 -> 1.9.0.

I'm filing this issue in case that wasn't an intentional backwards-incompatible breakage.

One example user is the ext4 package, which makes use of crc::crc32::update(). There is a pending pull-req to fix the ext4 crate, but as-is the build has been broken.

Code throws an error at benches\bench.rs at line 1:

Rust error:
error[E0554]: #![feature] may not be used on the stable release channel

Hello and thank you for the hard work you have put into this crate!

I noticed that the latest commits to the master branch are 8 months old and have not yet been released on crates.io. The new_custom() method has been added to the various digests since the last release, and I think it would be useful to have this method available via downloads from crates.io.

Would it be possible to create a new release? I would be more than happy to help out in anyway possible if there are outstanding issues that need to be resolved first.

On a related note, would you be okay with me opening a pull request to address clippy lint warnings? The changes necessary to resolve the warnings are very minimal and shouldn't require much effort to review.

Utilizing the crc crate with the crc64::Digest::new(crc64::ECMA) and core::Hasher interface, I was taken aback by the cost of it creating and allocating a table on the stack every time I start a new digest, which is prohibitive in an embedded environment.

The short-hand format checksum_ecma(...) is convenient only as long as data is available in a single run; I had to fall back to defining the field u64 and passing it through update, which is by far not as elegant as using the core::Hasher trait.

As I don't quite understand the reason for the different interfaces, I can make only uninformed suggestions on how the crate's behavior would be better suitable for my needs:

• There could be a slimmer type wrapper around the intermediates that makes the static tables of update() available with a core::Hasher interface.
• The Digest struct could become more general to allow building a Digest that uses the static tables.
• The various two interfaces could be told apart in the README; right now the examples read as if checksum_x25 were just a shorthand for the later crc16::Digest::new(crc16::X25) with write and sum16, while actually it's doing vastly different things performance-wise.

Such a table could also be generated (I believe) with something like this: (pseudocode)~~

for i in 0..256 {
    msb_first_table[i] = lsb_first_table[bitswap(i)]
}

EDIT: nope, there's more going on with it than that.

It's typically done as a seperate function though. Here's an untested example based off of your make_table implementation: (and random implementations elsewhere)

pub fn make_msb_table_crc64(poly: u64) -> [u64; 256] {
    let mut table = [0u64; 256];
    for i in 0..256 {
        let mut value = i as u64;
        for _ in 0..8 {
            value = if (value >> 63) == 1 {
                (value << 1) ^ poly
            } else {
                value << 1
            }
        }
        table[i] = value;
    }
    table
}

I'm having trouble getting the crc32 computation to match what I'm seeing in Ceph.
I have an example here where at the top is a C++ unit test and on the bottom is the Rust code I'm using to try and reproduce it. It's close but doesn't match.

https://play.rust-lang.org/?gist=6bca7e3630d48f9225e5&version=stable

Much of the code between crc32 and crc64 is identical, barring the different types.

This is most obvious in the table generation code in src/util.rs, where the implementations are side-by-side, but can also be seen in most, if not all of the code in the crc32 and crc64 modules.

I feel like this could be fixed with a function with a generic argument, but that would leave the question of, say, how to implement a CRC-5 or CRC-24... maybe some sort of n-bit integer type? BigInt except you specify the number of bits? That would probably be in the scope of another crate entirely, though, so long as the trait bounds were permissive enough.

When the cksum calculates its output, it would append a byte stream of octal numbers representing the length of the original input. However, using CRC_32_CKSUM alone does not imitate the behaviour and some additional logic is needed to get the same output as cksum.

A test program:

use crc;

const CKSUM: crc::Crc<u32> = crc::Crc::<u32>::new(&crc::CRC_32_CKSUM);

fn main() {
    let arg = std::env::args().nth(1).unwrap();
    let input = arg.as_bytes();
    let mut digest = CKSUM.digest();
    digest.update(input);
    let mut i = input.len();
    while i > 0 {
        let len_oct: u8 = (i & 0xFF).try_into().unwrap();
        digest.update(&[len_oct]);
        i >>= 8;
    }
    println!("No length: {}, has length: {}", CKSUM.checksum(input), digest.finalize());
}

Result:

> cargo run 123456789
No length: 1985902208, has length: 930766865
> printf 123456789 | cksum
930766865 9

Should this be considered a bug or a feature?

As mentioned in the previous issue, I'm trying to match crcs with LVM's C implementation. CRCs weren't matching until I removed the two negations of "value" in crc32.rs update(). Do those lines serve a purpose, or can they be removed safely?

It would be great to be able to omit the allocation of the tables in memory-constrained environments and instead fall back to generating the value for each byte on demand.

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?

If binary size is important, it's better to generate the CRC tables (once) at runtime instead of embedding them into the binary. This should be explained in the docs along with an example.

Hi, I was trying to use this crate on a #![no_std] system but I am getting errors that you are using the std crate.
However when looking at your code it seems that you are conditionally removing std usage unless the std feature is specified.

This brings me to the question, what am I missing? As from the code it should not be a problem as I have not specified the std feature in my dependencies on this crate.

Thanks!

How to calculate the well known CRC-16-CCITT with poly = 0x1021?
according to https://www.lammertbies.nl/comm/info/crc-calculation and http://srecord.sourceforge.net/crc16-ccitt.html, we shall get 0xE5CC or 0x29B1 for bytes b"123456789", but I could not manage to get it.
The combination I've tried the combination of poly in [0x8408, 0x1021] and init in [0, !0x1D0F], but with no luck.

  fn digest_crc(init: u16, poly: u16, input: &[u8]) -> u16 {
    use crc::crc16::Hasher16;
    let mut digest = crc::crc16::Digest::new_with_initial(poly, init);
    digest.write(input);
    !digest.sum16()
  }
  let input = b"123456789";
  digest_crc(0, 0x1021, input);
  digest_crc(0, 0x8408, input);
  digest_crc(!0x1D0F, 0x1021, input);
  digest_crc(!0x1D0F, 0x8408, input);

I even tried poly of [0x811, 0x8810] according to this https://en.wikipedia.org/wiki/Cyclic_redundancy_check.

To make use of:

• Compile-time assertions
• Const generics
• 2021 edition
• rust-version field in Cargo manifest

Is there a reason you chose to implement the CRC32 algorithm in a reversed order?

Everything from defining the polynomial to generating the lookup table to updating the CRC is reversed.

In build.rs IEEE is defined as 0xedb88320 which is the reflected form of the correct polynomial 0x04C11DB7. Then make_table_crc32() works in a Little Endian way.

I am currently using a custom polynomial and custom XOR value, and I cannot use your code in the current state. I have modified it to use the correct CRC32 algorithm. Are you interested in me making a pull request?

Is there a reason there isDigest::new instead of just:

• Digest::new_ieee()
• Digest::new_castagnoli()

where those methods just copy the already computed tables.

With #13 this could remove make_table from the library. If the user wants to use their own table, couldn't they compute it (using a new crate crc-table) and pass it in?

I'm willing to work on this.

This makes things easier for people searching for a CRC variant that matches some other existing code; compare https://docs.rs/crc/1.8.1/crc/crc32/constant.IEEE_TABLE.html with web.mit.edu/freebsd/head/sys/libkern/crc32.c.

In the comments of #37, the ability to specify both ref_in and ref_out was discussed. I also think this capability would be good to add. If this change is made, I think it would be good to update the implementations of Digest to follow the builder pattern, rather than adding even more parameters to the new_custom() function. In fact, as the new_custom() function hasn't been included in a release yet (as far as I can tell), it could even be removed.

For example, I envision something like the following:

// Anything not explicitly set is given a "sane" default value
let mut digest = crc16::Digest::new(poly)
    .initial_crc(crc)
    .reflection_in(true)
    .reflection_out(true);

Ultimately I don't have a strong preference on how these values are set, so feel free to "no change" this issue, but I would be more than happy to prototype the changes if this is something that might be wanted.

Context: #92 (comment)

Some advantages of --cfg flags:

• Don't have to be strictly additive
• Can only be set when compiling a binary (hidden to libraries)
• Can throw a compile error if incompatible flags are set
• Can stop supporting them without breaking semver

Pending tasks:

• Review the docs.
• Consider cutting a release candidate (e.g. 3.1.0-rc-0).
• Test upgrade some of the dependents.
• Add a CHANGELOG
• Resolve #93 (blocker)

Addresses a couple of different issues (#37, #46, #45, #40):

• code duplication under the crc16, crc32, and crc64 modules
• dependency on build scripts/macros (multiple Digests can share a crc table)

// Crc

pub trait Width: From<u8> + Into<u64> + Copy + 'static {}
impl Width for u16 {}
impl Width for u32 {}
impl Width for u64 {}

pub struct Algorithm<W: Width> {
    pub poly: W,
    pub init: W,
    pub xorout: W,
    pub check: W,
    pub residue: W,
    pub refin: bool,
    pub refout: bool,
}

pub const CRC_16_IBM_3740: Algorithm<u16> = Algorithm {
    poly: 0x1021, init: 0xffff,
    xorout: 0x0000, check: 0x29b1, residue: 0x0000,
    refin: false, refout: false,
};
pub const CRC_16_AUTOSAR: Algorithm<u16> = CRC_16_IBM_3740;

pub struct Crc<W: Width> {
    algorithm: &'static Algorithm<W>,
    table: [W; 256],
}

impl<W: Width> Crc<W> {
    fn new(algorithm: &'static Algorithm<W>) -> Self {
        let table = make_table(algorithm.poly, algorithm.init);
        Crc { algorithm, table }
    }
}

// Digest

pub struct Digest<'a, W: Width> {
    crc: &'a Crc<W>,
    value: W,
}

impl<'a, W: Width> Digest<'a, W> {
    fn new(crc: &'a Crc<W>) -> Self {
        let value = crc.algorithm.init;
        Digest { crc, value }
    }
}

impl<'a, W: Width> core::hash::Hasher for Digest<'a, W> {
    fn write(&mut self, bytes: &[u8]) {}
    fn finish(&self) -> u64 {
        unimplemented!()
    }
}

fn make_table<W: Width>(poly: W, init: W) -> [W; 256] {
    unimplemented!()
}

This 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.

Imagine a use-case where someone would like to maintain a rolling CRC digest, while also regularly reading the sum of that CRC digest via the equivalent of calling finalize. This is a common pattern used by write ahead logs where you want to maintain a rolling CRC of all the data written to the log, while also embedding the rolling finalized value in each of the records written to disk.

As far as I can tell, the current APIs are not compatible with this use-case:

• The higher-level Digest struct's finalize method unnecessarily consumes itself via it's self receiver, disallowing the interleaving of multiple calls to update and finalize.
• The lower level Crc structs don't make update or finalize public, disallowing consumes to write their own version of Digest that allows the interleaving of multiple calls to update and finalize.

A workaround might be to clone the Digest before calling finalize which seems inefficient.

Any thoughts on this use-case and the best way to unlock it?

Hi! We use your crate in a variety of embedded applications. I noticed that there's a recently landed, yet-unreleased commit changing the default time-vs-space tradeoff of the crate to use significantly larger tables.

Instead of making slice16 the default, what about making NoTable or the current behavior the default? The tables generated by this crate are already too large for some of our applications, and with this change we'd need to move away from the crate entirely.

(I'm aware that we could go through and update our code and all consumers to use NoTable decorators, etc., but that seems hard to achieve in practice in the presence of third-party code.)

Thanks!

This restricts how a program could construct a custom algorithm locally ands use it on the fly.

I noticed that the current internal of Crc holds the reference. Relaxing the lifetime means Crc would need a lifetime parameter as well, which is a breaking change unfortunately. An alternative way is to let Crc clone the data Algorithm has.

Digest should dervie Copy.

Usage:

struct ChunkedWriter<I, W> {
    crc: Digest<'static, I>,
    writer: W,
}

impl<I: Implementation, W: io::Write> io::Write for ChunkedWriter<I, W> {
    fn write(&mut self, mut buf: &[u8]) -> io::Result<usize> {
        // some other codes... end of a chunk
        let checksum = self.crc.finalize().to_be_bytes(); // <- cannot move out from a mutable reference
        self.writer.write_all(&checksum)?;
        self.crc = CRC.digest();

        // other...
    }
}

It would be nice to divide the load of calculation the CRC of multiple cores.

The function Combine in Ionic.Crc.CRC32 apparently can do this.

/// Combines the given CRC32 value with the current running total.
/// crc the crc value to be combined with this one
/// length the length of data the CRC value was calculated on</param>
public void Combine(int crc, int length);

https://github.com/r2d2rigo/dotnetzip-for-unity/blob/master/CommonSrc/CRC32.cs

• Should MSRV bump require a minor or major version?
• How many recent stable versions should be supported for a new release?

This is more of a question rather than an issue.

I have one line code written with v1 API:

let checksum = crc32::update(my_u32, &crc32::IEEE_TABLE, mybytes).to_be_bytes();

I tried to translate to V3 API:

let CRC_32_IEEE: Crc<u32> = Crc::<u32>::new(&CRC_32_ISO_HDLC);
let mut digest = CRC_32_IEEE.digest_with_initial(my_u32);
digest.update(mybytes);
let checksum = digest.finalize();

But the result has changed.

Any ideas? Thanks

The addition of NoTable is great; it's helpful for reclaiming a kilobyte or two on very constrained embedded systems. But, it only works if the user is vigilant about always doing Crc::<_, NoTable>::new() to specify the implementation they want. (Even if they use a OnceCell as suggested, nothing prevents someone elsewhere in the app or in a library from directly doing Crc::new() to get a default implementation.)

It would be nice if there were feature flags to enable implementations, so that a user could make NoTable the only available implementation, thereby preventing the larger Table<1> from creeping back into their binary by mistake.

Conversely, other users might want to make Table<16> the only available implementation, so that it's impossible to accidentally create a Crc using a slower Table<1> implementation.

Release 3.1.0 broke the build of lzma-rs (possibly among others). The reason seems to be the changed trait bound of crc::Digest:

• https://docs.rs/crc/3.1.0/crc/struct.Digest.html: pub struct Digest<'a, I: Implementation>
• https://docs.rs/crc/3.0.1/crc/struct.Digest.html: pub struct Digest<'a, W: Width>

As per my understanding this is not in line with semantic versioning requirements. Would it be possible to yank this version and fix this or publish it as 4.0 instead?

Thanks for being considerate of the wider eco system.

If we use include! we could remove the sub-crate pretty trivially:

https://blog.clap.rs/complete-me/#completionscriptgenerationinclap

// build.rs
extern crate clap;

use clap::Shell;

include!("src/cli.rs");

fn main() {  
    let mut app = build_cli();
    app.gen_completions("fake",          // We specify the bin name manually
                        Shell::Bash,      // Which shell to build completions for
                        env!("OUT_DIR")); // Where write the completions to
}

Basically the functions that we need can just be defined in files and then we can manaully include them for the build script -- removing the need of a sub-crate.

I unfortunately lack time for a full repro right now, but maybe this will help. This code:

       let crc = u32::from_le_bytes(data[0..CRC_SIZE].try_into().map_err(|_| Error::DataSize)?);
       let computed_crc = crc32::checksum_ieee(&data[DATA_START..]);

       if crc != computed_crc {
           return Err(Error::Crc);
       }

works right before that commit, but hits the error case after it.

I am trying to work with LVM metadata CRCs. Their crc calcs both use a nonstandard table, and also use an initial value of 0xf597a6cf instead of 0. I was able to work around this with a local copy of crc-rs and changing the value field in struct Digest to public, but wanted your thoughts on fixing it upstream.

for reference: https://git.fedorahosted.org/cgit/lvm2.git/tree/lib/misc/crc.c