kernel: utilities: add crc32b implementation #3825
Conversation
This is helpful for, for one example, computing ShortIDs from process names.
kernel/src/utilities/helpers.rs
Outdated
| /// | ||
| /// Implementation based on https://lxp32.github.io/docs/a-simple-example-crc32-calculation/. | ||
| /// Online calculator here: https://md5calc.com/hash/crc32b | ||
| pub fn crc32b_str(s: &'static str) -> u32 { |
There was a problem hiding this comment.
Might be nicer to have this accept a byte slice instead. This way we can use core::str::as_bytes to calculate a CRC over a string, but also support other non-unicode byte sequences. Shouldn't need a lot of changes, just iterating over the bytes and removing the cast.
|
Why not use the existing CRC implementation? |
I think adding a dependency of tickv for this use case is pretty heavyweight. Also the kernel won't have tickv as a dependency and I need to use this in the kernel. I don't actually need CRC, I just need something of |
|
You could copy the same implementation though. Keep all our CRCs the same |
I need something so I can do something like: ShortID::Fixed(NonZeroU32::new(kernel::utilities::helpers::crc("circle")).unwrap())Any extra machinery would just complicate that code for I don't think much benefit. |
|
I don't follow. You said in the PR description that this is helpful for computing shortIDs. Why do we need to use CRC-32B? Could we use CRC-32 to try to keep all of our CRCs the same? Maybe it's worth saying why we need this exact implementation in the PR description? It's not really clear |
|
I updated the PR description. I guess I liken this to something like addition. Sure, there are libraries that enable adding any two numbers together. But for simple cases the |
|
Version that matches tickv in #3829 |
|
I'm not really invested into which particular CRC polynomial we ship here, but is the intention to include this PR, #3829, or both? And if both are included, which one will we use for which purpose? Will TicKV ever use the one shipped in the kernel, and if not, what's the point of including it? |
|
I don't think anyone wants to add crc32b. But the PR description is still helpful. |
Pull Request Overview
Alternative to #3808.
This is helpful for, for one example, computing ShortIDs from process names.
The
Compresstrait has one function:I want to add a function with the signature
fn (&'static str) -> u32)so I can implementCompressand assign ShortIDs based on process names.This complements #3818, which is an app checker that looks like:
For my purposes (and for most simple use cases where apps are identified by their name), it's not important what the hash function is, as long as it is reasonably ok at generating different short IDs for different strings.
Now, I've been convinced to change the signature of the function in this PR to
fn (&[u8]) -> u32. That makes this a little more general. For my use case then I will create a small wrapper function in the board main.rs, or do something like&|s| crc32b(s.as_bytes()).The other reason I want any simple function like
fn (&'static str) -> u32)is it makes actually using AppID in board files fairly ergonomic. So If I have a capsule and I want to give some resource to a specific app, I can do something like:I guess now I have to do:
but you get the point.
What makes this nice is that it's easy for someone to know the ShortID for an app since we already use process names. Changing which app gets a resource is just updating the human readable string.
If we want to make AppID usable, having ways to generate ShortIDs that is intuitive is very helpful.
Testing Strategy
Comparing to online calculator.
TODO or Help Wanted
n/a
Documentation Updated
/docs, or no updates are required.Formatting
make prepush.