Tweaked lfsr_mtree_traversal_next to no longer write the mtree/mroot

This was a cludge due to needing lfs->mtree initialized to traverse the
mtree, the assumption being that future traversals should strictly
update the mtree/mroot to the existing state.

Moving code around (and adopting an actual state machine, which will be
needed for btree traversal) made this no longer necessary.

Now the mtree/mroot is only initialized in lfsr_mountinited, as it
should be.

lfs.c

@@ -2377,6 +2377,40 @@ static int lfsr_rbyd_fetch(lfs_t *lfs, lfsr_rbyd_t *rbyd,
     return 0;
 }
 
+// a more aggressive fetch when checksum is known
+static int lfsr_rbyd_fetchvalidate(lfs_t *lfs, lfsr_rbyd_t *rbyd,
+        lfs_block_t block, lfs_size_t trunk, lfsr_rid_t weight,
+        uint32_t cksum) {
+    int err = lfsr_rbyd_fetch(lfs, rbyd, block, trunk);
+    if (err) {
+        if (err == LFS_ERR_CORRUPT) {
+            LFS_ERROR("Found corrupted rbyd "
+                    "(0x%"PRIx32".%"PRIx32", 0x%08"PRIx32")",
+                    block, trunk, cksum);
+        }
+        return err;
+    }
+
+    // test that our cksum matches what's expected
+    //
+    // it should be noted that this is very unlikely to happen without the
+    // above fetch failing, since that would require the rbyd to have the
+    // same trunk and pass its internal cksum
+    if (rbyd->cksum != cksum) {
+        LFS_ERROR("Found rbyd cksum mismatch "
+                "(0x%"PRIx32".%"PRIx32", 0x%08"PRIx32" != 0x%08"PRIx32")",
+                rbyd->block, rbyd->trunk, rbyd->cksum, cksum);
+        return LFS_ERR_CORRUPT;
+    }
+
+    // if trunk/weight mismatch _after_ cksums match, that's not a storage
+    // error, that's a programming error
+    LFS_ASSERT(rbyd->trunk == trunk);
+    LFS_ASSERT((lfsr_rid_t)rbyd->weight == weight);
+    return 0;
+}
+
+
 static int lfsr_rbyd_lookupnext(lfs_t *lfs, const lfsr_rbyd_t *rbyd,
         lfsr_srid_t rid, lfsr_tag_t tag,
         lfsr_srid_t *rid_,
@@ -4218,7 +4252,8 @@ static int lfsr_btree_commit(lfs_t *lfs, lfsr_btree_t *btree,
         //
         // a funny benefit is we cache the root of our btree this way
         if (!lfsr_rbyd_isfetched(&rbyd)) {
-            int err = lfsr_rbyd_fetch(lfs, &rbyd, rbyd.block, rbyd.trunk);
+            int err = lfsr_rbyd_fetchvalidate(lfs, &rbyd,
+                    rbyd.block, rbyd.trunk, rbyd.weight, rbyd.cksum);
             if (err) {
                 return err;
             }
@@ -4926,7 +4961,6 @@ static int lfsr_btree_traversal_next(lfs_t *lfs, const lfsr_btree_t *btree,
 
 
 
-
 /// Metadata pair operations ///
 
 // the mroot anchor, mdir 0x{0,1} is the entry point into the filesystem
@@ -6823,6 +6857,7 @@ next:;
 typedef struct lfsr_mtree_traversal {
     // core traversal state
     uint8_t flags;
+    uint8_t state;
     lfsr_mdir_t mdir;
     union {
         // cycle detection state, only valid when mdir.mid.bid == -1
@@ -6837,13 +6872,25 @@ typedef struct lfsr_mtree_traversal {
 } lfsr_mtree_traversal_t;
 
 enum {
-    LFSR_MTREE_TRAVERSAL_VALIDATE = 0x1,
+    // traverse all blocks in the filesystem
+    LFSR_MTREE_TRAVERSAL_ALL = 0x1,
+    // validate checksums while traversing
+    LFSR_MTREE_TRAVERSAL_VALIDATE = 0x2,
+};
+
+// traversing littlefs is a bit complex, so we use a state machine to keep
+// track of where we are
+enum {
+    LFSR_MTREE_TRAVERSAL_MROOTANCHOR = 0,
+    LFSR_MTREE_TRAVERSAL_MROOTCHAIN  = 1,
+    LFSR_MTREE_TRAVERSAL_MTREE       = 2,
+    LFSR_MTREE_TRAVERSAL_BTREE       = 3,
 };
 
 #define LFSR_MTREE_TRAVERSAL(_flags) \
     ((lfsr_mtree_traversal_t){ \
         .flags=_flags, \
-        .mdir.u.m.trunk=0, \
+        .state=LFSR_MTREE_TRAVERSAL_MROOTANCHOR, \
         .u.tortoise.blocks={0, 0}, \
         .u.tortoise.step=0, \
         .u.tortoise.power=0})
@@ -6851,11 +6898,12 @@ enum {
 static int lfsr_mtree_traversal_next(lfs_t *lfs,
         lfsr_mtree_traversal_t *traversal,
         lfsr_smid_t *mid_, lfsr_tag_t *tag_, lfsr_data_t *data_) {
-    // new traversal? start with 0x{0,1}
+    switch (traversal->state) {
+    // start with the mrootanchor 0x{0,1}
     //
     // note we make sure to include all mroots in our mroot chain!
     //
-    if (traversal->mdir.u.m.trunk == 0) {
+    case LFSR_MTREE_TRAVERSAL_MROOTANCHOR:;
         // fetch the first mroot 0x{0,1}
         int err = lfsr_mdir_fetch(lfs, &traversal->mdir,
                 -1, LFSR_MBLOCKS_MROOTANCHOR());
@@ -6863,6 +6911,9 @@ static int lfsr_mtree_traversal_next(lfs_t *lfs,
             return err;
         }
 
+        // transition to traversing the mroot chain
+        traversal->state = LFSR_MTREE_TRAVERSAL_MROOTCHAIN;
+
         if (mid_) {
             *mid_ = -1;
         }
@@ -6874,21 +6925,22 @@ static int lfsr_mtree_traversal_next(lfs_t *lfs,
         }
         return 0;
 
-    // check for mroot/mtree/mdir
+    // traverse the mroot chain, checking for mroot/mtree/mdir
-    } else if (traversal->mdir.mid == -1) {
+    case LFSR_MTREE_TRAVERSAL_MROOTCHAIN:;
         // lookup mroot, if we find one this is a fake mroot
         lfsr_tag_t tag;
         lfsr_data_t data;
-        int err = lfsr_mdir_lookup(lfs, &traversal->mdir,
+        err = lfsr_mdir_lookup(lfs, &traversal->mdir,
                 -1, LFSR_TAG_WIDE(STRUCT),
                 &tag, &data);
-        if (err && err != LFS_ERR_NOENT) {
+        if (err) {
             return err;
         }
 
         // found a new mroot
-        if (err != LFS_ERR_NOENT && tag == LFSR_TAG_MROOT) {
+        if (tag == LFSR_TAG_MROOT) {
-            err = lfsr_data_readmblocks(lfs, &data, traversal->mdir.u.m.blocks);
+            err = lfsr_data_readmblocks(lfs, &data,
+                    traversal->mdir.u.m.blocks);
             if (err) {
                 return err;
             }
@@ -6938,41 +6990,107 @@ static int lfsr_mtree_traversal_next(lfs_t *lfs,
             }
             return 0;
 
-        // no more mroots, which makes this our real mroot
+        // found an mdir?
-        } else {
-            // update our mroot
-            lfs->mroot = traversal->mdir;
-
-            // do we have an mtree? mdir?
-            if (err == LFS_ERR_NOENT) {
-                lfs->mtree = LFSR_BTREE_NULL;
         } else if (tag == LFSR_TAG_MDIR) {
+            // fetch this mdir
+            err = lfsr_data_readmblocks(lfs, &data,
+                    traversal->mdir.u.m.blocks);
+            if (err) {
+                return err;
+            }
+
+            err = lfsr_mdir_fetch(lfs, &traversal->mdir,
+                    0, traversal->mdir.u.m.blocks);
+            if (err) {
+                return err;
+            }
+
+            // TODO this is ugly
+            // transition to traversing the mtree, but skip our mdir
+            traversal->state = LFSR_MTREE_TRAVERSAL_MTREE;
+            lfsr_btree_t mtree = LFSR_BTREE_NULL;
             err = lfsr_data_readbtreeinlined(lfs, &data,
                     LFSR_TAG_MDIR, lfsr_mleafweight(lfs),
-                        &lfs->mtree);
+                    &mtree);
             if (err) {
                 return err;
             }
+            traversal->u.traversal = LFSR_BTREE_TRAVERSAL();
+            err = lfsr_btree_traversal_next(
+                    lfs, &mtree,
+                    &traversal->u.traversal,
+                    NULL, NULL, NULL, NULL);
+            if (err) {
+                return err;
+            }
+
+            if (mid_) {
+                *mid_ = 0;
+            }
+            if (tag_) {
+                *tag_ = LFSR_TAG_MDIR;
+            }
+            if (data_) {
+                *data_ = LFSR_DATA_BUF(&traversal->mdir, sizeof(lfsr_mdir_t));
+            }
+            return 0;
+
+        // found an mtree?
         } else if (tag == LFSR_TAG_MTREE) {
-                err = lfsr_data_readbtree(lfs, &data, &lfs->mtree.u.rbyd);
+            // read the root of the mtree and return it, lfs->mtree may not
+            // be initialized yet
+            // TODO uh, should we make traversal->mdir a different type?
+            err = lfsr_data_readbtree(lfs, &data, &traversal->mdir.u.rbyd);
             if (err) {
                 return err;
             }
+
+            // validate our btree nodes if requested, this just means we need
+            // to do a full rbyd fetch and make sure the checksums match
+            if (traversal->flags & LFSR_MTREE_TRAVERSAL_VALIDATE) {
+                err = lfsr_rbyd_fetchvalidate(lfs, &traversal->mdir.u.rbyd,
+                        traversal->mdir.u.rbyd.block,
+                        traversal->mdir.u.rbyd.trunk,
+                        traversal->mdir.u.rbyd.weight,
+                        traversal->mdir.u.rbyd.cksum);
+                if (err) {
+                    return err;
+                }
+            }
+
+            // transition to traversing the mtree, but skip the root
+            traversal->state = LFSR_MTREE_TRAVERSAL_MTREE;
+            traversal->u.traversal = LFSR_BTREE_TRAVERSAL();
+            err = lfsr_btree_traversal_next(
+                    lfs, (lfsr_btree_t*)&traversal->mdir.u.rbyd,
+                    &traversal->u.traversal,
+                    NULL, NULL, NULL, NULL);
+            if (err) {
+                return err;
+            }
+
+            if (mid_) {
+                *mid_ = 0;
+            }
+            if (tag_) {
+                *tag_ = LFSR_TAG_BTREE;
+            }
+            if (data_) {
+                *data_ = LFSR_DATA_BUF(&traversal->mdir.u.rbyd,
+                        sizeof(lfsr_rbyd_t));
+            }
+            return 0;
+
         } else {
-                LFS_ERROR("Weird mstruct? (0x%"PRIx32")", tag);
+            LFS_ERROR("Weird mtree entry? (0x%"PRIx32")", tag);
             return LFS_ERR_CORRUPT;
         }
 
-            // initialize our mtree traversal
+    // traverse the mtree, including both inner btree nodes and mdirs
-            traversal->u.traversal = LFSR_BTREE_TRAVERSAL();
+    case LFSR_MTREE_TRAVERSAL_MTREE:;
-        }
-    }
-
         // traverse through the mtree
         lfsr_bid_t bid;
-    lfsr_tag_t tag;
+        err = lfsr_btree_traversal_next(
-    lfsr_data_t data;
-    int err = lfsr_btree_traversal_next(
                 lfs, &lfs->mtree, &traversal->u.traversal,
                 &bid, &tag, NULL, &data);
         if (err) {
@@ -6985,49 +7103,19 @@ static int lfsr_mtree_traversal_next(lfs_t *lfs,
             // to do a full rbyd fetch and make sure the checksums match
             if (traversal->flags & LFSR_MTREE_TRAVERSAL_VALIDATE) {
                 lfsr_rbyd_t *branch = (lfsr_rbyd_t*)data.u.direct.buffer;
-            lfsr_rbyd_t branch_;
+                err = lfsr_rbyd_fetchvalidate(lfs, branch,
-            err = lfsr_rbyd_fetch(lfs, &branch_,
+                        branch->block, branch->trunk, branch->weight,
-                    branch->block, branch->trunk);
+                        branch->cksum);
                 if (err) {
-                if (err == LFS_ERR_CORRUPT) {
-                    LFS_ERROR("Corrupted rbyd during mtree traversal "
-                            "(0x%"PRIx32".%"PRIx32", 0x%08"PRIx32")",
-                            branch->block, branch->trunk, branch->cksum);
-                }
                     return err;
                 }
-
-            // test that our branch's cksum matches what's expected
-            //
-            // it should be noted it's very unlikely for this to be hit without
-            // the above fetch failing since it includes both an internal
-            // cksum check and trunk check
-            if (branch_.cksum != branch->cksum) {
-                LFS_ERROR("Checksum mismatch during mtree traversal "
-                        "(0x%"PRIx32".%"PRIx32", "
-                        "0x%08"PRIx32" != 0x%08"PRIx32")",
-                        branch->block, branch->trunk,
-                        branch_.cksum, branch->cksum);
-                return LFS_ERR_CORRUPT;
             }
 
-            LFS_ASSERT(branch_.trunk == branch->trunk);
-            LFS_ASSERT(branch_.weight == branch->weight);
-
-            // TODO is this useful at all?
-            // change our branch to the fetched version
-            *branch = branch_;
-        }
-
-        // still update our mdir mid so we don't get stuck in a loop
-        // traversing mroots
-        traversal->mdir.mid = bid;
-
             if (mid_) {
                 *mid_ = bid;
             }
             if (tag_) {
-            *tag_ = tag;
+                *tag_ = LFSR_TAG_BTREE;
             }
             if (data_) {
                 *data_ = data;
@@ -7036,7 +7124,8 @@ static int lfsr_mtree_traversal_next(lfs_t *lfs,
 
         // fetch mdir if we're on a leaf
         } else if (tag == LFSR_TAG_MDIR) {
-        err = lfsr_data_readmblocks(lfs, &data, traversal->mdir.u.m.blocks);
+            err = lfsr_data_readmblocks(lfs, &data,
+                    traversal->mdir.u.m.blocks);
             if (err) {
                 return err;
             }
@@ -7051,7 +7140,7 @@ static int lfsr_mtree_traversal_next(lfs_t *lfs,
                 *mid_ = bid;
             }
             if (tag_) {
-            *tag_ = tag;
+                *tag_ = LFSR_TAG_MDIR;
             }
             if (data_) {
                 *data_ = LFSR_DATA_BUF(&traversal->mdir, sizeof(lfsr_mdir_t));
@@ -7062,6 +7151,16 @@ static int lfsr_mtree_traversal_next(lfs_t *lfs,
             LFS_ERROR("Weird mtree entry? (0x%"PRIx32")", tag);
             return LFS_ERR_CORRUPT;
         }
+
+    // traverse any file btree, including both inner btree nodes and
+    // block pointers
+    case LFSR_MTREE_TRAVERSAL_BTREE:;
+        // TODO
+        return 0;
+
+    default:;
+        LFS_UNREACHABLE();
+    }
 }
 
 
@@ -7153,6 +7252,10 @@ static int lfsr_mountinited(lfs_t *lfs) {
     // zero gdeltas, we'll read these from our mdirs
     lfsr_fs_flushgdelta(lfs);
 
+    // default to no mtree, this is allowed and implies all files are inlined
+    // in the mroot
+    lfs->mtree = LFSR_BTREE_NULL;
+
     // traverse the mtree rooted at mroot 0x{1,0}
     //
     // note that lfsr_mtree_traversal_next will update our mroot/mtree
@@ -7175,12 +7278,10 @@ static int lfsr_mountinited(lfs_t *lfs) {
             return err;
         }
 
-        // we only care about mdirs here
+        // found an mdir?
-        if (tag != LFSR_TAG_MDIR) {
+        if (tag == LFSR_TAG_MDIR) {
-            continue;
-        }
-
             lfsr_mdir_t *mdir = (lfsr_mdir_t*)data.u.direct.buffer;
+
             // found an mroot?
             if (mdir->mid == -1) {
                 // has magic string?
@@ -7218,7 +7319,8 @@ static int lfsr_mountinited(lfs_t *lfs) {
                 }
 
                 uint32_t major_version;
-            err = lfsr_data_readleb128(lfs, &data, (int32_t*)&major_version);
+                err = lfsr_data_readleb128(lfs, &data,
+                        (int32_t*)&major_version);
                 if (err && err != LFS_ERR_CORRUPT) {
                     return err;
                 }
@@ -7227,7 +7329,8 @@ static int lfsr_mountinited(lfs_t *lfs) {
                 }
 
                 uint32_t minor_version;
-            err = lfsr_data_readleb128(lfs, &data, (int32_t*)&minor_version);
+                err = lfsr_data_readleb128(lfs, &data,
+                        (int32_t*)&minor_version);
                 if (err && err != LFS_ERR_CORRUPT) {
                     return err;
                 }
@@ -7278,7 +7381,8 @@ static int lfsr_mountinited(lfs_t *lfs) {
 
                 uint32_t cksum_type = 0;
                 if (err != LFS_ERR_NOENT) {
-                err = lfsr_data_readleb128(lfs, &data, (int32_t*)&cksum_type);
+                    err = lfsr_data_readleb128(lfs, &data,
+                            (int32_t*)&cksum_type);
                     if (err && err != LFS_ERR_CORRUPT) {
                         return err;
                     }
@@ -7301,7 +7405,8 @@ static int lfsr_mountinited(lfs_t *lfs) {
 
                 uint32_t redund_type = 0;
                 if (err != LFS_ERR_NOENT) {
-                err = lfsr_data_readleb128(lfs, &data, (int32_t*)&redund_type);
+                    err = lfsr_data_readleb128(lfs, &data,
+                            (int32_t*)&redund_type);
                     if (err && err != LFS_ERR_CORRUPT) {
                         return err;
                     }
@@ -7311,7 +7416,8 @@ static int lfsr_mountinited(lfs_t *lfs) {
                 }
 
                 if (redund_type != 0) {
-                LFS_ERROR("Incompatible redund type 0x%"PRId32, redund_type);
+                    LFS_ERROR("Incompatible redund type 0x%"PRId32,
+                            redund_type);
                     return LFS_ERR_INVAL;
                 }
 
@@ -7324,7 +7430,8 @@ static int lfsr_mountinited(lfs_t *lfs) {
 
                 uint32_t block_limit = 0;
                 if (err != LFS_ERR_NOENT) {
-                err = lfsr_data_readleb128(lfs, &data, (int32_t*)&block_limit);
+                    err = lfsr_data_readleb128(lfs, &data,
+                            (int32_t*)&block_limit);
                     if (err && err != LFS_ERR_CORRUPT) {
                         return err;
                     }
@@ -7350,7 +7457,8 @@ static int lfsr_mountinited(lfs_t *lfs) {
 
                 uint32_t disk_limit = 0;
                 if (err != LFS_ERR_NOENT) {
-                err = lfsr_data_readleb128(lfs, &data, (int32_t*)&disk_limit);
+                    err = lfsr_data_readleb128(lfs, &data,
+                            (int32_t*)&disk_limit);
                     if (err && err != LFS_ERR_CORRUPT) {
                         return err;
                     }
@@ -7390,7 +7498,8 @@ static int lfsr_mountinited(lfs_t *lfs) {
                 // we only support power-of-two mleaf weights, this unlikely to
                 // ever to change since mleaf weights are pretty arbitrary
                 if (lfs_popc(mleaf_limit+1) != 1) {
-                LFS_ERROR("Incompatible mleaf weight %"PRId32, mleaf_limit+1);
+                    LFS_ERROR("Incompatible mleaf weight %"PRId32,
+                            mleaf_limit+1);
                     return LFS_ERR_INVAL;
                 }
 
@@ -7463,7 +7572,8 @@ static int lfsr_mountinited(lfs_t *lfs) {
 
                 if (err != LFS_ERR_NOENT) {
                     uint32_t utag_limit;
-                err = lfsr_data_readleb128(lfs, &data, (int32_t*)&utag_limit);
+                    err = lfsr_data_readleb128(lfs, &data,
+                            (int32_t*)&utag_limit);
                     if (err && err != LFS_ERR_CORRUPT) {
                         return err;
                     }
@@ -7490,7 +7600,8 @@ static int lfsr_mountinited(lfs_t *lfs) {
 
                 if (err != LFS_ERR_NOENT) {
                     uint32_t uattr_limit;
-                err = lfsr_data_readleb128(lfs, &data, (int32_t*)&uattr_limit);
+                    err = lfsr_data_readleb128(lfs, &data,
+                            (int32_t*)&uattr_limit);
                     if (err && err != LFS_ERR_CORRUPT) {
                         return err;
                     }
@@ -7507,6 +7618,22 @@ static int lfsr_mountinited(lfs_t *lfs) {
                         return LFS_ERR_INVAL;
                     }
                 }
+
+                // keep track of the last mroot we see, this is the "real" mroot
+                lfs->mroot = *mdir;
+
+            } else {
+                // found a direct mdir? keep track of this as our "mtree"
+                if (lfsr_btree_isnull(&lfs->mtree)) {
+                    // TODO this works but is super weird, get rid of inlined
+                    // btrees?
+                    uint8_t mdir_buf[LFSR_MDIR_DSIZE];
+                    err = lfsr_btree_commit(lfs, &lfs->mtree, LFSR_ATTRS(
+                            LFSR_ATTR(0,
+                                MDIR, +lfsr_mleafweight(lfs),
+                                FROMMBLOCKS(mdir->u.m.blocks, mdir_buf))));
+                    LFS_ASSERT(!err);
+                }
             }
 
             // collect any gdeltas from this mdir
@@ -7514,6 +7641,19 @@ static int lfsr_mountinited(lfs_t *lfs) {
             if (err) {
                 return err;
             }
+
+        // found an mtree inner-node?
+        } else if (tag == LFSR_TAG_BTREE) {
+            lfsr_rbyd_t *branch = (lfsr_rbyd_t*)data.u.direct.buffer;
+
+            // found the root of the mtree?
+            if (lfsr_btree_isnull(&lfs->mtree)) {
+                lfs->mtree.u.rbyd = *branch;
+            }
+
+        } else {
+            LFS_UNREACHABLE();
+        }
     }
 
     // once we've mounted and derived a pseudo-random seed, initialize our
@@ -7754,6 +7894,9 @@ static int lfs_alloc(lfs_t *lfs, lfs_block_t *block) {
             } else if (tag == LFSR_TAG_BTREE) {
                 lfsr_rbyd_t *branch = (lfsr_rbyd_t*)data.u.direct.buffer;
                 lfs_alloc_setinuse(lfs, branch->block);
+
+            } else {
+                LFS_UNREACHABLE();
             }
         }
     }