Made data read functions "consume" their data pointers
Composable parsing functions always feel a bit weird to me in C. I don't
know if this is because of something C lacks, such as multiple return
values, or if composable parsers are just inherently awkward to describe
in procedural languages because of the different levels of state.
But I think the API here is pretty ok. The main idea is that data
parsers can be added as functions in the lfsr_data_* namespace that take
lfsr_data_t as a mutable reference, updating the lfsr_data_t's internal
state as data is parsed.
In practice you only need a couple of primitives, bytes, le32s, leb128s,
that touch the internals of lfsr_data_t, and the other parsers can be
built using these.
This leverages the pointer-like abstraction of lfsr_data_t, and avoids
needing to keep track of offsets. And thanks to lfsr_data_t being
relatively cheap to make copies, this API is relatively flexible.
Some other tweaks:
- Signed leb128 overflow detection is moved up into lfs_fromleb128.
littlefs now assumes _all_ leb128s are 31-bits, which is useful for
leveraging the sign bit internally.
This also fixes the an issue in overflow detection in lfs_fromleb128
which wouldn't catch overflows in the last byte of a >32-bit leb128.
- Most lfsr_data_t functions now take a pointer. This offered a small
bit of code savings and feels more natural in C. Though most functions
that accept lfsr_data_t still take a copy. Most of these functions
would need to make a copy anyways now that the parsers are consuming,
and these copies avoid concerns about shared state.
At 3-words, lfsr_data_t is right at that boundary of questionable
reasonableness for copying, but copying is a very useful feature of
this struct.
This ends up with some decent code/stack savings:
code stack
before: 22118 2048
after: 21722 (-1.8%) 1992 (-2.7%)
This commit is contained in:
+9
-4
@@ -16,7 +16,7 @@
|
||||
|
||||
|
||||
// Convert to/from leb128 encoding
|
||||
ssize_t lfs_toleb128(uint32_t word, void *buffer, size_t size) {
|
||||
ssize_t lfs_toleb128(int32_t word, void *buffer, size_t size) {
|
||||
uint8_t *data = buffer;
|
||||
|
||||
for (size_t i = 0; i < size; i++) {
|
||||
@@ -33,14 +33,19 @@ ssize_t lfs_toleb128(uint32_t word, void *buffer, size_t size) {
|
||||
return LFS_ERR_CORRUPT;
|
||||
}
|
||||
|
||||
ssize_t lfs_fromleb128(uint32_t *word, const void *buffer, size_t size) {
|
||||
ssize_t lfs_fromleb128(int32_t *word, const void *buffer, size_t size) {
|
||||
const uint8_t *data = buffer;
|
||||
|
||||
uint32_t word_ = 0;
|
||||
int32_t word_ = 0;
|
||||
for (size_t i = 0; i < size; i++) {
|
||||
uint8_t dat = data[i];
|
||||
int32_t dat = data[i];
|
||||
word_ |= (dat & 0x7f) << 7*i;
|
||||
if (!(dat & 0x80)) {
|
||||
// did we overflow?
|
||||
if ((word_ >> 7*i) != dat) {
|
||||
return LFS_ERR_CORRUPT;
|
||||
}
|
||||
|
||||
*word = word_;
|
||||
return i+1;
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user