littlefs-project / littlefs

A little fail-safe filesystem designed for microcontrollers
BSD 3-Clause "New" or "Revised" License
4.9k stars 770 forks source link

Allow CRC-32 match value override #988

Closed jmalmari closed 1 week ago

jmalmari commented 1 month ago

LFS format failed because CRC=0 is, in the general case, a wrong value to test against. For example, valid zlib CRC-32 match value would be 0x2144df1c.

geky-bot commented 1 month ago
Tests passed ✓, Code: 17064 B (+0.0%), Stack: 1440 B (+0.0%), Structs: 812 B (+0.0%) | | Code | Stack | Structs | | Coverage | |:--|-----:|------:|--------:|:--|---------:| | Default | 17064 B (+0.0%) | 1440 B (+0.0%) | 812 B (+0.0%) | Lines | 2394/2574 lines (-0.0%) | | Readonly | 6194 B (+0.0%) | 448 B (+0.0%) | 812 B (+0.0%) | Branches | 1245/1584 branches (+0.0%) | | Threadsafe | 17924 B (+0.0%) | 1440 B (+0.0%) | 820 B (+0.0%) | | **Benchmarks** | | Multiversion | 17124 B (+0.0%) | 1440 B (+0.0%) | 816 B (+0.0%) | Readed | 29369693876 B (+0.0%) | | Migrate | 18760 B (+0.0%) | 1744 B (+0.0%) | 816 B (+0.0%) | Proged | 1482874766 B (+0.0%) | | Error-asserts | 17748 B (+0.0%) | 1432 B (+0.0%) | 812 B (+0.0%) | Erased | 1568888832 B (+0.0%) |
geky-bot commented 4 weeks ago
Tests passed ✓, Code: 17064 B (+0.0%), Stack: 1440 B (+0.0%), Structs: 812 B (+0.0%) | | Code | Stack | Structs | | Coverage | |:--|-----:|------:|--------:|:--|---------:| | Default | 17064 B (+0.0%) | 1440 B (+0.0%) | 812 B (+0.0%) | Lines | 2394/2574 lines (-0.0%) | | Readonly | 6194 B (+0.0%) | 448 B (+0.0%) | 812 B (+0.0%) | Branches | 1245/1584 branches (+0.0%) | | Threadsafe | 17924 B (+0.0%) | 1440 B (+0.0%) | 820 B (+0.0%) | | **Benchmarks** | | Multiversion | 17124 B (+0.0%) | 1440 B (+0.0%) | 816 B (+0.0%) | Readed | 29369693876 B (+0.0%) | | Migrate | 18760 B (+0.0%) | 1744 B (+0.0%) | 816 B (+0.0%) | Proged | 1482874766 B (+0.0%) | | Error-asserts | 17748 B (+0.0%) | 1432 B (+0.0%) | 812 B (+0.0%) | Erased | 1568888832 B (+0.0%) |
geky commented 1 week ago

Hi @jmalmari, thanks for creating a PR.

This is probably equivalent to inverting the crc before and after calculation:

uint32_t lfs_crc(uint32_t crc, const void *buffer, size_t size) {
    return ~my_crc(~crc, buffer, size);
}

If you're looking to use a different polynomial/checksum, that's a bit of a different question that would create compatibility issues for other littlefs tooling. Required a source-level change is probably warranted in this case for now.

jmalmari commented 1 week ago

Right, I did not spot that connection. User side change is probably better. Takes only a few extra cycles.

geky commented 1 week ago

To be fair, the inverted form is more common, since it can also detect prefixed zero bytes.

Long term, it would be nice to move to the inverted form (or even a better polynomial, such as crc32c), but since this would require disk-breaking changes, it's sort of out of the question.