From 76ffb0e7b6252f7ddbe94fc17e843634de17a5c4 Mon Sep 17 00:00:00 2001
From: Christopher Haster <geky@geky.net>
Date: Wed, 5 Jun 2024 00:02:32 -0500
Subject: [PATCH] Fixed mroot death returning LFS_ERR_CORRUPT -> LFS_ERR_NOSPC

It's counter-intuitive, but no top-level API should return
LFS_ERR_CORRUPT. Instead, if we can't make progress because of a corrupt
block, we should return LFS_ERR_NOSPC. This makes it easier for users to
write code that is well behaved even when a device is end-of-life.

It's up to our mroot extension algorithm to make sure this case can't be
reached in normal operation unless the device is _actually_ at
end-of-life.

Because mroot extension is a bit of a special case, we weren't
converting these corrupt errors to nospc errors consistently. This is
fixed now, along with a couple more hopefully-useful logging statements.

Found while playing around with test_exhaustion + block_recycles=-1.
This should assert on bad wear-leveling, but LFS_ERR_CORRUPT was
unexpected. Added an explicit test because this is an easy thing to let
split through:

- test_badblocks_mrootanchor_wear

Code changes were surprisingly minimal, I wonder if constants are being
swapped out somewhere low-level?

           code          stack
  before: 33766           2600
  after:  33770 (+0.0%)   2600 (+0.0%)
---
 lfs.c                     | 21 +++++++++++++++--
 tests/test_badblocks.toml | 49 ++++++++++++++++++++++++++++++++++++++-
 2 files changed, 67 insertions(+), 3 deletions(-)

diff --git a/lfs.c b/lfs.c
index feb7d001..4de2a901 100644
--- a/lfs.c
+++ b/lfs.c
@@ -6668,9 +6668,12 @@ relocate:;
 
             err = lfsr_mdir_swap__(lfs, &mdir_, mdir, true);
             if (err) {
-                // bad prog? try another block
+                // bad prog? can't do much here, mdir stuck
                 if (err == LFS_ERR_CORRUPT) {
-                    goto relocate;
+                    LFS_DEBUG("Stuck mdir 0x{%"PRIx32",%"PRIx32"}",
+                            mdir->rbyd.blocks[0],
+                            mdir->rbyd.blocks[1]);
+                    return LFS_ERR_NOSPC;
                 }
                 return err;
             }
@@ -7211,6 +7214,13 @@ static int lfsr_mdir_commit(lfs_t *lfs, lfsr_mdir_t *mdir,
             lfsr_mdir_t mrootanchor_;
             err = lfsr_mdir_swap__(lfs, &mrootanchor_, &mrootchild, true);
             if (err) {
+                // bad prog? can't do much here, mroot stuck
+                if (err == LFS_ERR_CORRUPT) {
+                    LFS_DEBUG("Stuck mroot 0x{%"PRIx32",%"PRIx32"}",
+                            mrootanchor_.rbyd.blocks[0],
+                            mrootanchor_.rbyd.blocks[1]);
+                    return LFS_ERR_NOSPC;
+                }
                 goto failed;
             }
 
@@ -7228,6 +7238,13 @@ static int lfsr_mdir_commit(lfs_t *lfs, lfsr_mdir_t *mdir,
             if (err) {
                 LFS_ASSERT(err != LFS_ERR_RANGE);
                 LFS_ASSERT(err != LFS_ERR_NOENT);
+                // bad prog? can't do much here, mroot stuck
+                if (err == LFS_ERR_CORRUPT) {
+                    LFS_DEBUG("Stuck mroot 0x{%"PRIx32",%"PRIx32"}",
+                            mrootanchor_.rbyd.blocks[0],
+                            mrootanchor_.rbyd.blocks[1]);
+                    return LFS_ERR_NOSPC;
+                }
                 goto failed;
             }
         }
diff --git a/tests/test_badblocks.toml b/tests/test_badblocks.toml
index 38ac33d6..4492ae46 100644
--- a/tests/test_badblocks.toml
+++ b/tests/test_badblocks.toml
@@ -5095,7 +5095,7 @@ code = '''
 ## other corner cases
 
 # test formatting with 0 or 1 bad, this should just error
-[cases.test_badblocks_mrootanchor]
+[cases.test_badblocks_mrootanchor_format]
 defines.ERASE_CYCLES = 0xffffffff
 defines.BADBLOCKS = [0x1, 0x2, 0x3]
 defines.BADBLOCK_BEHAVIOR = [
@@ -5119,6 +5119,53 @@ code = '''
     lfsr_format(&lfs, CFG) => LFS_ERR_CORRUPT;
 '''
 
+# test blocks 0 or 1 going bad, this should just error
+[cases.test_badblocks_mrootanchor_wear]
+defines.ERASE_CYCLES = 0xffffffff
+defines.BADBLOCKS = [0x1, 0x2]
+defines.BADBLOCK_BEHAVIOR = [
+    'LFS_EMUBD_BADBLOCK_PROGERROR',
+    'LFS_EMUBD_BADBLOCK_ERASEERROR',
+    'LFS_EMUBD_BADBLOCK_READERROR',
+    'LFS_EMUBD_BADBLOCK_PROGNOOP',
+    'LFS_EMUBD_BADBLOCK_ERASENOOP',
+]
+# we need prog checking to detect read errors
+defines.CHECK_PROGS = 'BADBLOCK_BEHAVIOR >= LFS_EMUBD_BADBLOCK_READERROR'
+code = '''
+    lfs_t lfs;
+    lfsr_format(&lfs, CFG) => 0;
+
+    if (BADBLOCKS & 0x1) {
+        lfs_emubd_setwear(CFG, 0, 0xffffffff) => 0;
+    }
+    if (BADBLOCKS & 0x2) {
+        lfs_emubd_setwear(CFG, 1, 0xffffffff) => 0;
+    }
+
+    lfsr_mount(&lfs, CFG) => 0;
+
+    for (lfs_size_t i = 0;; i++) {
+        // this should eventually fail
+        assert(i <= BLOCK_COUNT);
+
+        int err = lfsr_mkdir(&lfs, "hi");
+        assert(!err || err == LFS_ERR_NOSPC);
+        if (err == LFS_ERR_NOSPC) {
+            break;
+        }
+
+        err = lfsr_remove(&lfs, "hi");
+        assert(!err || err == LFS_ERR_NOSPC);
+        if (err == LFS_ERR_NOSPC) {
+            break;
+        }
+    }
+
+    lfsr_unmount(&lfs) => 0;
+'''
+
+