Github/littlefs
1
Fork 0
Code Actions Activity

Prefer unsigned ints when using sign-bit as a flag

Browse Source 
Note this is slightly different than cases where we use the sign-bit for
muxing two different types, such as `int err` and `lfsr_srid_t rid`. In
those cases we'd never extract the lower bits of the int
unconditionally.

This leads to fewer casts and I think signals the intention of these
sign-bit-is-flag ints a bit better. We aren't really interpreting these
as signed, and mask out other bits in some cases (lfsr_data_t).

This leads to more code in places, I'm guessing because of C treating
signed overflow as undefined behavior... Maybe this is a good thing:

           code          stack
  before: 33856           2824
  after:  33876 (+0.1%)   2824 (+0.0%)
This commit is contained in:
Christopher Haster
2024-05-02 14:38:11 -05:00
parent add985a3f4
commit 580ff26024
2 changed files with 36 additions and 41 deletions
Download Patch File Download Diff File Expand all files Collapse all files
lfs.c
+24 -29
View File
@@ -1780,7 +1780,7 @@ static lfsr_data_t lfsr_data_fromecksum(const lfsr_ecksum_t *ecksum,
// you shouldn't try to encode a not-ecksum, that doesn't make sense // you shouldn't try to encode a not-ecksum, that doesn't make sense
LFS_ASSERT(ecksum->cksize != -1); LFS_ASSERT(ecksum->cksize != -1);
// cksize should not exceed 28-bits // cksize should not exceed 28-bits
LFS_ASSERT((uint32_t)ecksum->cksize <= 0x0fffffff); LFS_ASSERT((lfs_size_t)ecksum->cksize <= 0x0fffffff);
lfs_ssize_t d = 0; lfs_ssize_t d = 0;
lfs_ssize_t d_ = lfs_toleb128(ecksum->cksize, &buffer[d], 4); lfs_ssize_t d_ = lfs_toleb128(ecksum->cksize, &buffer[d], 4);
@@ -1795,7 +1795,7 @@ static lfsr_data_t lfsr_data_fromecksum(const lfsr_ecksum_t *ecksum,
static int lfsr_data_readecksum(lfs_t *lfs, lfsr_data_t *data, static int lfsr_data_readecksum(lfs_t *lfs, lfsr_data_t *data,
lfsr_ecksum_t *ecksum) { lfsr_ecksum_t *ecksum) {
int err = lfsr_data_readlleb128(lfs, data, (uint32_t*)&ecksum->cksize); int err = lfsr_data_readlleb128(lfs, data, (lfs_size_t*)&ecksum->cksize);
if (err) { if (err) {
return err; return err;
} }
@@ -1868,8 +1868,7 @@ static lfsr_data_t lfsr_data_frombptr(const lfsr_bptr_t *bptr,
static int lfsr_data_readbptr(lfs_t *lfs, lfsr_data_t *data, static int lfsr_data_readbptr(lfs_t *lfs, lfsr_data_t *data,
lfsr_bptr_t *bptr) { lfsr_bptr_t *bptr) {
// read the block, offset, size // read the block, offset, size
int err = lfsr_data_readlleb128(lfs, data, int err = lfsr_data_readlleb128(lfs, data, &bptr->data.u.disk.size);
(uint32_t*)&bptr->data.u.disk.size);
if (err) { if (err) {
return err; return err;
} }
@@ -2095,11 +2094,11 @@ static int lfsr_data_readgrm(lfs_t *lfs, lfsr_data_t *data,
} }
for (uint8_t i = 0; i < mode; i++) { for (uint8_t i = 0; i < mode; i++) {
int err = lfsr_data_readleb128(lfs, data, (uint32_t*)&grm->rms[i]); int err = lfsr_data_readleb128(lfs, data, (lfsr_mid_t*)&grm->rms[i]);
if (err) { if (err) {
return err; return err;
} }
LFS_ASSERT((uint32_t)grm->rms[i] < lfs_max32( LFS_ASSERT((lfsr_mid_t)grm->rms[i] < lfs_max32(
lfsr_mtree_weight(&lfs->mtree), lfsr_mtree_weight(&lfs->mtree),
lfsr_mleafweight(lfs))); lfsr_mleafweight(lfs)));
} }
@@ -2185,7 +2184,7 @@ static inline bool lfsr_rbyd_isfetched(const lfsr_rbyd_t *rbyd) {
} }
static inline bool lfsr_rbyd_parity(const lfsr_rbyd_t *rbyd) { static inline bool lfsr_rbyd_parity(const lfsr_rbyd_t *rbyd) {
return (lfs_size_t)rbyd->eoff >> (8*sizeof(lfs_size_t)-1); return rbyd->eoff >> (8*sizeof(lfs_size_t)-1);
} }
static inline lfs_size_t lfsr_rbyd_eoff(const lfsr_rbyd_t *rbyd) { static inline lfs_size_t lfsr_rbyd_eoff(const lfsr_rbyd_t *rbyd) {
@@ -2215,7 +2214,7 @@ static int lfsr_rbyd_alloc(lfs_t *lfs, lfsr_rbyd_t *rbyd) {
} }
static int lfsr_rbyd_fetch(lfs_t *lfs, lfsr_rbyd_t *rbyd, static int lfsr_rbyd_fetch(lfs_t *lfs, lfsr_rbyd_t *rbyd,
lfs_block_t block, lfs_ssize_t trunk) { lfs_block_t block, lfs_size_t trunk) {
// set up some initial state // set up some initial state
rbyd->blocks[0] = block; rbyd->blocks[0] = block;
rbyd->trunk = (trunk & LFSR_RBYD_ISSHRUB) | 0; rbyd->trunk = (trunk & LFSR_RBYD_ISSHRUB) | 0;
@@ -2246,7 +2245,7 @@ static int lfsr_rbyd_fetch(lfs_t *lfs, lfsr_rbyd_t *rbyd,
// scan tags, checking valid bits, cksums, etc // scan tags, checking valid bits, cksums, etc
while (off < lfs->cfg->block_size while (off < lfs->cfg->block_size
&& (!trunk || lfsr_rbyd_eoff(rbyd) <= (lfs_size_t)trunk)) { && (!trunk || lfsr_rbyd_eoff(rbyd) <= trunk)) {
lfsr_tag_t tag; lfsr_tag_t tag;
lfsr_rid_t weight__; lfsr_rid_t weight__;
lfs_size_t size; lfs_size_t size;
@@ -2347,7 +2346,7 @@ static int lfsr_rbyd_fetch(lfs_t *lfs, lfsr_rbyd_t *rbyd,
// found a trunk of a tree? // found a trunk of a tree?
if (lfsr_tag_istrunk(tag) if (lfsr_tag_istrunk(tag)
&& (!trunk || off <= (lfs_size_t)trunk || trunk__)) { && (!trunk || off <= trunk || trunk__)) {
// start of trunk? // start of trunk?
if (!trunk__) { if (!trunk__) {
// keep track of trunk's entry point // keep track of trunk's entry point
@@ -2369,7 +2368,7 @@ static int lfsr_rbyd_fetch(lfs_t *lfs, lfsr_rbyd_t *rbyd,
// update data checksum // update data checksum
cksum = cksum_; cksum = cksum_;
// update trunk and weight, unless we are a shrub trunk // update trunk and weight, unless we are a shrub trunk
if (!lfsr_tag_isshrub(tag) || trunk__ == (lfs_size_t)trunk) { if (!lfsr_tag_isshrub(tag) || trunk__ == trunk) {
trunk_ = trunk__; trunk_ = trunk__;
weight = weight_; weight = weight_;
} }
@@ -2411,7 +2410,7 @@ static int lfsr_rbyd_fetch(lfs_t *lfs, lfsr_rbyd_t *rbyd,
if (err != LFS_ERR_CORRUPT) { if (err != LFS_ERR_CORRUPT) {
ecksum_ = lfs_crc32c(0, &e, 1); ecksum_ = lfs_crc32c(0, &e, 1);
} }
int err = lfsr_bd_cksum(lfs, err = lfsr_bd_cksum(lfs,
rbyd->blocks[0], lfsr_rbyd_eoff(rbyd)+1, 0, rbyd->blocks[0], lfsr_rbyd_eoff(rbyd)+1, 0,
ecksum.cksize-1, ecksum.cksize-1,
&ecksum_); &ecksum_);
@@ -2432,7 +2431,7 @@ static int lfsr_rbyd_fetch(lfs_t *lfs, lfsr_rbyd_t *rbyd,
// a more aggressive fetch when checksum is known // a more aggressive fetch when checksum is known
static int lfsr_rbyd_fetchvalidate(lfs_t *lfs, lfsr_rbyd_t *rbyd, static int lfsr_rbyd_fetchvalidate(lfs_t *lfs, lfsr_rbyd_t *rbyd,
lfs_block_t block, lfs_ssize_t trunk, lfsr_rid_t weight, lfs_block_t block, lfs_size_t trunk, lfsr_rid_t weight,
uint32_t cksum) { uint32_t cksum) {
int err = lfsr_rbyd_fetch(lfs, rbyd, block, trunk); int err = lfsr_rbyd_fetch(lfs, rbyd, block, trunk);
if (err) { if (err) {
@@ -2458,7 +2457,7 @@ static int lfsr_rbyd_fetchvalidate(lfs_t *lfs, lfsr_rbyd_t *rbyd,
// if trunk/weight mismatch _after_ cksums match, that's not a storage // if trunk/weight mismatch _after_ cksums match, that's not a storage
// error, that's a programming error // error, that's a programming error
LFS_ASSERT(lfsr_rbyd_trunk(rbyd) == (lfs_size_t)trunk); LFS_ASSERT(lfsr_rbyd_trunk(rbyd) == trunk);
LFS_ASSERT(rbyd->weight == weight); LFS_ASSERT(rbyd->weight == weight);
return 0; return 0;
} }
@@ -2764,7 +2763,7 @@ static int lfsr_rbyd_p_flush(lfs_t *lfs, lfsr_rbyd_t *rbyd,
static inline int lfsr_rbyd_p_push(lfs_t *lfs, lfsr_rbyd_t *rbyd, static inline int lfsr_rbyd_p_push(lfs_t *lfs, lfsr_rbyd_t *rbyd,
lfsr_alt_t p[static 3], lfsr_alt_t p[static 3],
lfsr_tag_t alt, lfsr_srid_t weight, lfs_size_t jump) { lfsr_tag_t alt, lfsr_rid_t weight, lfs_size_t jump) {
int err = lfsr_rbyd_p_flush(lfs, rbyd, p, 1); int err = lfsr_rbyd_p_flush(lfs, rbyd, p, 1);
if (err) { if (err) {
return err; return err;
@@ -4144,7 +4143,7 @@ static int lfsr_data_readbranch(lfs_t *lfs, lfsr_data_t *data,
return err; return err;
} }
err = lfsr_data_readlleb128(lfs, data, (uint32_t*)&branch->trunk); err = lfsr_data_readlleb128(lfs, data, &branch->trunk);
if (err) { if (err) {
return err; return err;
} }
@@ -5283,7 +5282,7 @@ static int lfsr_data_readshrub(lfs_t *lfs, lfsr_data_t *data,
return err; return err;
} }
err = lfsr_data_readlleb128(lfs, data, (uint32_t*)&shrub->trunk); err = lfsr_data_readlleb128(lfs, data, &shrub->trunk);
if (err) { if (err) {
return err; return err;
} }
@@ -5319,7 +5318,7 @@ static lfs_ssize_t lfsr_shrub_estimate(lfs_t *lfs,
static int lfsr_shrub_compact(lfs_t *lfs, lfsr_rbyd_t *rbyd_, static int lfsr_shrub_compact(lfs_t *lfs, lfsr_rbyd_t *rbyd_,
lfsr_shrub_t *shrub_, const lfsr_shrub_t *shrub) { lfsr_shrub_t *shrub_, const lfsr_shrub_t *shrub) {
// save our current trunk/weight // save our current trunk/weight
lfs_ssize_t trunk = rbyd_->trunk; lfs_size_t trunk = rbyd_->trunk;
lfsr_srid_t weight = rbyd_->weight; lfsr_srid_t weight = rbyd_->weight;
// compact our bshrub // compact our bshrub
@@ -5368,7 +5367,7 @@ static int lfsr_shrub_commit(lfs_t *lfs, lfsr_rbyd_t *rbyd_,
// things up too much // things up too much
// //
// it is important that these rbyds share eoff/cksum/etc // it is important that these rbyds share eoff/cksum/etc
lfs_ssize_t trunk = rbyd_->trunk; lfs_size_t trunk = rbyd_->trunk;
lfsr_srid_t weight = rbyd_->weight; lfsr_srid_t weight = rbyd_->weight;
rbyd_->trunk = shrub->trunk; rbyd_->trunk = shrub->trunk;
rbyd_->weight = shrub->weight; rbyd_->weight = shrub->weight;
@@ -5693,7 +5692,7 @@ static bool lfsr_mid_isopen(lfs_t *lfs, lfsr_smid_t mid) {
: LFSR_BTREE_DSIZE) : LFSR_BTREE_DSIZE)
static inline bool lfsr_mtree_isnull(const lfsr_mtree_t *mtree) { static inline bool lfsr_mtree_isnull(const lfsr_mtree_t *mtree) {
return (lfsr_mid_t)mtree->u.weight == (LFSR_MTREE_ISMPTR | 0); return mtree->u.weight == (LFSR_MTREE_ISMPTR | 0);
} }
static inline bool lfsr_mtree_ismptr(const lfsr_mtree_t *mtree) { static inline bool lfsr_mtree_ismptr(const lfsr_mtree_t *mtree) {
@@ -8063,14 +8062,12 @@ static lfsr_data_t lfsr_data_fromgeometry(const lfsr_geometry_t *geometry,
static int lfsr_data_readgeometry(lfs_t *lfs, lfsr_data_t *data, static int lfsr_data_readgeometry(lfs_t *lfs, lfsr_data_t *data,
lfsr_geometry_t *geometry) { lfsr_geometry_t *geometry) {
int err = lfsr_data_readlleb128(lfs, data, int err = lfsr_data_readlleb128(lfs, data, &geometry->block_size);
(uint32_t*)&geometry->block_size);
if (err) { if (err) {
return err; return err;
} }
err = lfsr_data_readleb128(lfs, data, err = lfsr_data_readleb128(lfs, data, &geometry->block_count);
(uint32_t*)&geometry->block_count);
if (err) { if (err) {
return err; return err;
} }
@@ -8196,7 +8193,7 @@ static int lfsr_mountmroot(lfs_t *lfs, const lfsr_mdir_t *mroot) {
} }
// read the name limit // read the name limit
uint32_t name_limit = 0xff; lfs_size_t name_limit = 0xff;
err = lfsr_mdir_lookup(lfs, mroot, LFSR_TAG_NAMELIMIT, err = lfsr_mdir_lookup(lfs, mroot, LFSR_TAG_NAMELIMIT,
&data); &data);
if (err && err != LFS_ERR_NOENT) { if (err && err != LFS_ERR_NOENT) {
@@ -8222,7 +8219,7 @@ static int lfsr_mountmroot(lfs_t *lfs, const lfsr_mdir_t *mroot) {
lfs->name_limit = name_limit; lfs->name_limit = name_limit;
// read the size limit // read the size limit
uint32_t size_limit = 0x7fffffff; lfs_off_t size_limit = 0x7fffffff;
err = lfsr_mdir_lookup(lfs, mroot, LFSR_TAG_SIZELIMIT, err = lfsr_mdir_lookup(lfs, mroot, LFSR_TAG_SIZELIMIT,
&data); &data);
if (err && err != LFS_ERR_NOENT) { if (err && err != LFS_ERR_NOENT) {
@@ -8348,9 +8345,7 @@ static int lfsr_mountinited(lfs_t *lfs) {
} }
// check for any orphaned files // check for any orphaned files
for (lfs_size_t rid = 0; for (lfs_size_t rid = 0; rid < tinfo.u.mdir.rbyd.weight; rid++) {
rid < (lfs_size_t)tinfo.u.mdir.rbyd.weight;
rid++) {
err = lfsr_rbyd_lookup(lfs, &tinfo.u.mdir.rbyd, err = lfsr_rbyd_lookup(lfs, &tinfo.u.mdir.rbyd,
rid, LFSR_TAG_ORPHAN, rid, LFSR_TAG_ORPHAN,
NULL); NULL);
lfs.h
+12 -12
View File
@@ -343,12 +343,12 @@ typedef struct lfsr_rbyd {
lfs_block_t blocks[2]; lfs_block_t blocks[2];
// sign(trunk)=0 => normal rbyd // sign(trunk)=0 => normal rbyd
// sign(trunk)=1 => shrub rbyd // sign(trunk)=1 => shrub rbyd
lfs_ssize_t trunk; lfs_size_t trunk;
// sign(eoff) => commit parity // sign(eoff) => commit parity
// eoff=0, trunk=0 => not yet committed // eoff=0, trunk=0 => not yet committed
// eoff=0, trunk>0 => not yet fetched // eoff=0, trunk>0 => not yet fetched
// eoff>=block_size => rbyd not erased/needs compaction // eoff>=block_size => rbyd not erased/needs compaction
lfs_ssize_t eoff; lfs_size_t eoff;
uint32_t cksum; uint32_t cksum;
} lfsr_rbyd_t; } lfsr_rbyd_t;
@@ -360,7 +360,7 @@ typedef struct {
// this mostly lines up with lfsr_rbyd_t // this mostly lines up with lfsr_rbyd_t
lfsr_rid_t weight; lfsr_rid_t weight;
lfs_block_t blocks[2]; lfs_block_t blocks[2];
lfs_ssize_t trunk; lfs_size_t trunk;
// except for shrub estimate, which takes the place of eoff, etc // except for shrub estimate, which takes the place of eoff, etc
lfs_size_t estimate; lfs_size_t estimate;
} lfsr_shrub_t; } lfsr_shrub_t;
@@ -406,22 +406,22 @@ typedef struct lfs_mdir {
// //
typedef struct lfsr_data { typedef struct lfsr_data {
union { union {
lfs_ssize_t size; lfs_size_t size;
struct { struct {
lfs_ssize_t size; lfs_size_t size;
lfs_block_t block; lfs_block_t block;
lfs_size_t off; lfs_size_t off;
} disk; } disk;
struct { struct {
lfs_ssize_t size; lfs_size_t size;
const uint8_t *buffer; const uint8_t *buffer;
} buf; } buf;
struct { struct {
lfs_ssize_t size; lfs_size_t size;
uint8_t buf[8]; uint8_t buf[8];
} imm; } imm;
struct { struct {
lfs_ssize_t size; lfs_size_t size;
const struct lfsr_data *datas; const struct lfsr_data *datas;
} cat; } cat;
} u; } u;
@@ -442,7 +442,7 @@ typedef struct lfsr_dir {
lfsr_opened_t p; // pos mdir lfsr_opened_t p; // pos mdir
lfsr_opened_t b; // bookmark mdir lfsr_opened_t b; // bookmark mdir
lfsr_did_t did; lfsr_did_t did;
lfs_soff_t pos; lfs_off_t pos;
} lfsr_dir_t; } lfsr_dir_t;
// littlefs file type // littlefs file type
@@ -486,7 +486,7 @@ typedef struct lfsr_bshrub {
// sign(size)=0, data.block!=mdir.block => btree // sign(size)=0, data.block!=mdir.block => btree
// //
union { union {
lfs_soff_t size; lfs_off_t size;
lfsr_sprout_t bsprout; lfsr_sprout_t bsprout;
lfsr_bptr_t bptr; lfsr_bptr_t bptr;
lfsr_shrub_t bshrub; lfsr_shrub_t bshrub;
@@ -531,9 +531,9 @@ typedef struct lfsr_mtree {
union { union {
// the sign bit indicates if this is an inlined mdir/direct mdir // the sign bit indicates if this is an inlined mdir/direct mdir
// pointer or a full mtree // pointer or a full mtree
lfsr_smid_t weight; lfsr_mid_t weight;
struct { struct {
lfsr_smid_t weight; lfsr_mid_t weight;
lfsr_mptr_t mptr; lfsr_mptr_t mptr;
} mptr; } mptr;
lfsr_btree_t btree; lfsr_btree_t btree;