Merge pull request #811 from littlefs-project/fix-deorphan-repeatedly

Fix issue where lfs_fs_deorphan may run more than needed

lfs.c

@@ -4741,8 +4741,6 @@ static int lfs_fs_deorphan(lfs_t *lfs, bool powerloss) {
         return 0;
     }
 
-    int8_t found = 0;
-
     // Check for orphans in two separate passes:
     // - 1 for half-orphans (relocations)
     // - 2 for full-orphans (removes/renames)
@@ -4813,8 +4811,6 @@ static int lfs_fs_deorphan(lfs_t *lfs, bool powerloss) {
                             return state;
                         }
 
-                        found += 1;
-
                         // did our commit create more orphans?
                         if (state == LFS_OK_ORPHANED) {
                             moreorphans = true;
@@ -4849,8 +4845,6 @@ static int lfs_fs_deorphan(lfs_t *lfs, bool powerloss) {
                         return state;
                     }
 
-                    found += 1;
-
                     // did our commit create more orphans?
                     if (state == LFS_OK_ORPHANED) {
                         moreorphans = true;
@@ -4868,9 +4862,7 @@ static int lfs_fs_deorphan(lfs_t *lfs, bool powerloss) {
     }
 
     // mark orphans as fixed
-    return lfs_fs_preporphans(lfs, -lfs_min(
+    return lfs_fs_preporphans(lfs, -lfs_gstate_getorphans(&lfs->gstate));
-            lfs_gstate_getorphans(&lfs->gstate),
-            found));
 }
 #endif
 

tests/test_orphans.toml

@@ -59,6 +59,73 @@ code = '''
     lfs_unmount(&lfs) => 0;
 '''
 
+# test that we only run deorphan once per power-cycle
+[cases.test_orphans_no_orphans]
+in = 'lfs.c'
+code = '''
+    lfs_t lfs;
+    lfs_format(&lfs, cfg) => 0;
+
+    lfs_mount(&lfs, cfg) => 0;
+    // mark the filesystem as having orphans
+    lfs_fs_preporphans(&lfs, +1) => 0;
+    lfs_mdir_t mdir;
+    lfs_dir_fetch(&lfs, &mdir, (lfs_block_t[2]){0, 1}) => 0;
+    lfs_dir_commit(&lfs, &mdir, NULL, 0) => 0;
+
+    // we should have orphans at this state
+    assert(lfs_gstate_hasorphans(&lfs.gstate));
+    lfs_unmount(&lfs) => 0;
+
+    // mount
+    lfs_mount(&lfs, cfg) => 0;
+    // we should detect orphans
+    assert(lfs_gstate_hasorphans(&lfs.gstate));
+    // force consistency
+    lfs_fs_forceconsistency(&lfs) => 0;
+    // we should no longer have orphans
+    assert(!lfs_gstate_hasorphans(&lfs.gstate));
+
+    lfs_unmount(&lfs) => 0;
+'''
+
+[cases.test_orphans_one_orphan]
+in = 'lfs.c'
+code = '''
+    lfs_t lfs;
+    lfs_format(&lfs, cfg) => 0;
+
+    lfs_mount(&lfs, cfg) => 0;
+    // create an orphan
+    lfs_mdir_t orphan;
+    lfs_alloc_ack(&lfs);
+    lfs_dir_alloc(&lfs, &orphan) => 0;
+    lfs_dir_commit(&lfs, &orphan, NULL, 0) => 0;
+
+    // append our orphan and mark the filesystem as having orphans
+    lfs_fs_preporphans(&lfs, +1) => 0;
+    lfs_mdir_t mdir;
+    lfs_dir_fetch(&lfs, &mdir, (lfs_block_t[2]){0, 1}) => 0;
+    lfs_pair_tole32(orphan.pair);
+    lfs_dir_commit(&lfs, &mdir, LFS_MKATTRS(
+            {LFS_MKTAG(LFS_TYPE_SOFTTAIL, 0x3ff, 8), orphan.pair})) => 0;
+
+    // we should have orphans at this state
+    assert(lfs_gstate_hasorphans(&lfs.gstate));
+    lfs_unmount(&lfs) => 0;
+
+    // mount
+    lfs_mount(&lfs, cfg) => 0;
+    // we should detect orphans
+    assert(lfs_gstate_hasorphans(&lfs.gstate));
+    // force consistency
+    lfs_fs_forceconsistency(&lfs) => 0;
+    // we should no longer have orphans
+    assert(!lfs_gstate_hasorphans(&lfs.gstate));
+
+    lfs_unmount(&lfs) => 0;
+'''
+
 # reentrant testing for orphans, basically just spam mkdir/remove
 [cases.test_orphans_reentrant]
 reentrant = true