Added handling for corrupt as initial state of blocks

Before this, littlefs incorrectly assumed corrupt blocks were only the result
of our own modification. This would be fine for most cases of freshly
erased storage, but for storage with block-level ECC this wasn't always
true.

Fortunately, it's quite easy for littlefs to handle this case correctly,
as long as corrupt storage always reports that it is corrupt, which for
most forms of ECC is the case unless we perform a write on the storage.

found by rojer

.travis.yml

@@ -27,7 +27,7 @@ script:
   # compile and find the code size with the smallest configuration
   - make clean size
         OBJ="$(ls lfs*.o | tr '\n' ' ')"
-        CFLAGS+="-DLFS_NO{ASSERT,DEBUG,WARN,ERROR}"
+        CFLAGS+="-DLFS_NO_ASSERT -DLFS_NO_DEBUG -DLFS_NO_WARN -DLFS_NO_ERROR"
         | tee sizes
 
   # update status if we succeeded, compare with master if possible

Makefile

@@ -1,4 +1,7 @@
-TARGET = lfs
+TARGET = lfs.a
+ifneq ($(wildcard test.c main.c),)
+override TARGET = lfs
+endif
 
 CC ?= gcc
 AR ?= ar
@@ -22,7 +25,7 @@ ifdef WORD
 override CFLAGS += -m$(WORD)
 endif
 override CFLAGS += -I.
-override CFLAGS += -std=c99 -Wall -pedantic
+override CFLAGS += -std=c99 -Wall -pedantic -Wshadow -Wunused-parameter
 
 
 all: $(TARGET)
@@ -35,7 +38,9 @@ size: $(OBJ)
 .SUFFIXES:
 test: test_format test_dirs test_files test_seek test_truncate \
 	test_interspersed test_alloc test_paths test_orphan test_move test_corrupt
+	@rm test.c
 test_%: tests/test_%.sh
+
 ifdef QUIET
 	@./$< | sed -n '/^[-=]/p'
 else
@@ -44,7 +49,7 @@ endif
 
 -include $(DEP)
 
-$(TARGET): $(OBJ)
+lfs: $(OBJ)
 	$(CC) $(CFLAGS) $^ $(LFLAGS) -o $@
 
 %.a: $(OBJ)

emubd/lfs_emubd.c

@@ -16,6 +16,7 @@
 #include <unistd.h>
 #include <assert.h>
 #include <stdbool.h>
+#include <inttypes.h>
 
 
 // Block device emulated on existing filesystem
@@ -85,7 +86,7 @@ int lfs_emubd_read(const struct lfs_config *cfg, lfs_block_t block,
     memset(data, 0, size);
 
     // Read data
-    snprintf(emu->child, LFS_NAME_MAX, "%x", block);
+    snprintf(emu->child, LFS_NAME_MAX, "%" PRIx32, block);
 
     FILE *f = fopen(emu->path, "rb");
     if (!f && errno != ENOENT) {
@@ -124,7 +125,7 @@ int lfs_emubd_prog(const struct lfs_config *cfg, lfs_block_t block,
     assert(block < cfg->block_count);
 
     // Program data
-    snprintf(emu->child, LFS_NAME_MAX, "%x", block);
+    snprintf(emu->child, LFS_NAME_MAX, "%" PRIx32, block);
 
     FILE *f = fopen(emu->path, "r+b");
     if (!f) {
@@ -171,7 +172,7 @@ int lfs_emubd_erase(const struct lfs_config *cfg, lfs_block_t block) {
     assert(block < cfg->block_count);
 
     // Erase the block
-    snprintf(emu->child, LFS_NAME_MAX, "%x", block);
+    snprintf(emu->child, LFS_NAME_MAX, "%" PRIx32, block);
     struct stat st;
     int err = stat(emu->path, &st);
     if (err && errno != ENOENT) {
@@ -239,4 +240,3 @@ int lfs_emubd_sync(const struct lfs_config *cfg) {
 
     return 0;
 }
-

emubd/lfs_emubd.h

@@ -10,6 +10,11 @@
 #include "lfs.h"
 #include "lfs_util.h"
 
+#ifdef __cplusplus
+extern "C"
+{
+#endif
+
 
 // Config options
 #ifndef LFS_EMUBD_READ_SIZE
@@ -75,4 +80,8 @@ int lfs_emubd_erase(const struct lfs_config *cfg, lfs_block_t block);
 int lfs_emubd_sync(const struct lfs_config *cfg);
 
 
+#ifdef __cplusplus
+} /* extern "C" */
+#endif
+
 #endif

lfs.c

@@ -7,6 +7,8 @@
 #include "lfs.h"
 #include "lfs_util.h"
 
+#include <inttypes.h>
+
 
 /// Caching block device operations ///
 static int lfs_cache_read(lfs_t *lfs, lfs_cache_t *rcache,
@@ -107,6 +109,19 @@ static int lfs_cache_crc(lfs_t *lfs, lfs_cache_t *rcache,
     return 0;
 }
 
+static inline void lfs_cache_drop(lfs_t *lfs, lfs_cache_t *rcache) {
+    // do not zero, cheaper if cache is readonly or only going to be
+    // written with identical data (during relocates)
+    (void)lfs;
+    rcache->block = 0xffffffff;
+}
+
+static inline void lfs_cache_zero(lfs_t *lfs, lfs_cache_t *pcache) {
+    // zero to avoid information leak
+    memset(pcache->buffer, 0xff, lfs->cfg->prog_size);
+    pcache->block = 0xffffffff;
+}
+
 static int lfs_cache_flush(lfs_t *lfs,
         lfs_cache_t *pcache, lfs_cache_t *rcache) {
     if (pcache->block != 0xffffffff) {
@@ -128,7 +143,7 @@ static int lfs_cache_flush(lfs_t *lfs,
             }
         }
 
-        pcache->block = 0xffffffff;
+        lfs_cache_zero(lfs, pcache);
     }
 
     return 0;
@@ -233,7 +248,7 @@ static int lfs_bd_erase(lfs_t *lfs, lfs_block_t block) {
 }
 
 static int lfs_bd_sync(lfs_t *lfs) {
-    lfs->rcache.block = 0xffffffff;
+    lfs_cache_drop(lfs, &lfs->rcache);
 
     int err = lfs_cache_flush(lfs, &lfs->pcache, NULL);
     if (err) {
@@ -295,7 +310,8 @@ static int lfs_alloc(lfs_t *lfs, lfs_block_t *block) {
 
         // check if we have looked at all blocks since last ack
         if (lfs->free.ack == 0) {
-            LFS_WARN("No more free space %d", lfs->free.i + lfs->free.off);
+            LFS_WARN("No more free space %" PRIu32,
+                    lfs->free.i + lfs->free.off);
             return LFS_ERR_NOSPC;
         }
 
@@ -401,11 +417,14 @@ static int lfs_dir_alloc(lfs_t *lfs, lfs_dir_t *dir) {
     // rather than clobbering one of the blocks we just pretend
     // the revision may be valid
     int err = lfs_bd_read(lfs, dir->pair[0], 0, &dir->d.rev, 4);
-    dir->d.rev = lfs_fromle32(dir->d.rev);
-    if (err) {
+    if (err && err != LFS_ERR_CORRUPT) {
         return err;
     }
 
+    if (err != LFS_ERR_CORRUPT) {
+        dir->d.rev = lfs_fromle32(dir->d.rev);
+    }
+
     // set defaults
     dir->d.rev += 1;
     dir->d.size = sizeof(dir->d)+4;
@@ -429,6 +448,9 @@ static int lfs_dir_fetch(lfs_t *lfs,
         int err = lfs_bd_read(lfs, tpair[i], 0, &test, sizeof(test));
         lfs_dir_fromle32(&test);
         if (err) {
+            if (err == LFS_ERR_CORRUPT) {
+                continue;
+            }
             return err;
         }
 
@@ -448,6 +470,9 @@ static int lfs_dir_fetch(lfs_t *lfs,
         err = lfs_bd_crc(lfs, tpair[i], sizeof(test),
                 (0x7fffffff & test.size) - sizeof(test), &crc);
         if (err) {
+            if (err == LFS_ERR_CORRUPT) {
+                continue;
+            }
             return err;
         }
 
@@ -465,7 +490,8 @@ static int lfs_dir_fetch(lfs_t *lfs,
     }
 
     if (!valid) {
-        LFS_ERROR("Corrupted dir pair at %d %d", tpair[0], tpair[1]);
+        LFS_ERROR("Corrupted dir pair at %" PRIu32 " %" PRIu32 ,
+                tpair[0], tpair[1]);
         return LFS_ERR_CORRUPT;
     }
 
@@ -588,15 +614,16 @@ static int lfs_dir_commit(lfs_t *lfs, lfs_dir_t *dir,
         break;
 relocate:
         //commit was corrupted
-        LFS_DEBUG("Bad block at %d", dir->pair[0]);
+        LFS_DEBUG("Bad block at %" PRIu32, dir->pair[0]);
 
         // drop caches and prepare to relocate block
         relocated = true;
-        lfs->pcache.block = 0xffffffff;
+        lfs_cache_drop(lfs, &lfs->pcache);
 
         // can't relocate superblock, filesystem is now frozen
         if (lfs_paircmp(oldpair, (const lfs_block_t[2]){0, 1}) == 0) {
-            LFS_WARN("Superblock %d has become unwritable", oldpair[0]);
+            LFS_WARN("Superblock %" PRIu32 " has become unwritable",
+                    oldpair[0]);
             return LFS_ERR_CORRUPT;
         }
 
@@ -609,7 +636,7 @@ relocate:
 
     if (relocated) {
         // update references if we relocated
-        LFS_DEBUG("Relocating %d %d to %d %d",
+        LFS_DEBUG("Relocating %" PRIu32 " %" PRIu32 " to %" PRIu32 " %" PRIu32,
                 oldpair[0], oldpair[1], dir->pair[0], dir->pair[1]);
         int err = lfs_relocate(lfs, oldpair, dir->pair);
         if (err) {
@@ -836,7 +863,7 @@ nextname:
 
         // find entry matching name
         while (true) {
-            int err = lfs_dir_next(lfs, dir, entry);
+            err = lfs_dir_next(lfs, dir, entry);
             if (err) {
                 return err;
             }
@@ -1214,10 +1241,10 @@ static int lfs_ctz_extend(lfs_t *lfs,
         }
 
 relocate:
-        LFS_DEBUG("Bad block at %d", nblock);
+        LFS_DEBUG("Bad block at %" PRIu32, nblock);
 
         // just clear cache and try a new block
-        pcache->block = 0xffffffff;
+        lfs_cache_drop(lfs, &lfs->pcache);
     }
 }
 
@@ -1322,7 +1349,6 @@ int lfs_file_open(lfs_t *lfs, lfs_file_t *file,
     }
 
     // allocate buffer if needed
-    file->cache.block = 0xffffffff;
     if (lfs->cfg->file_buffer) {
         if (lfs->files) {
             // already in use
@@ -1341,6 +1367,9 @@ int lfs_file_open(lfs_t *lfs, lfs_file_t *file,
         }
     }
 
+    // zero to avoid information leak
+    lfs_cache_zero(lfs, &file->cache);
+
     // add to list of files
     file->next = lfs->files;
     lfs->files = file;
@@ -1369,7 +1398,7 @@ int lfs_file_close(lfs_t *lfs, lfs_file_t *file) {
 
 static int lfs_file_relocate(lfs_t *lfs, lfs_file_t *file) {
 relocate:
-    LFS_DEBUG("Bad block at %d", file->block);
+    LFS_DEBUG("Bad block at %" PRIu32, file->block);
 
     // just relocate what exists into new block
     lfs_block_t nblock;
@@ -1409,7 +1438,7 @@ relocate:
     memcpy(file->cache.buffer, lfs->pcache.buffer, lfs->cfg->prog_size);
     file->cache.block = lfs->pcache.block;
     file->cache.off = lfs->pcache.off;
-    lfs->pcache.block = 0xffffffff;
+    lfs_cache_zero(lfs, &lfs->pcache);
 
     file->block = nblock;
     return 0;
@@ -1418,7 +1447,7 @@ relocate:
 static int lfs_file_flush(lfs_t *lfs, lfs_file_t *file) {
     if (file->flags & LFS_F_READING) {
         // just drop read cache
-        file->cache.block = 0xffffffff;
+        lfs_cache_drop(lfs, &file->cache);
         file->flags &= ~LFS_F_READING;
     }
 
@@ -1433,7 +1462,7 @@ static int lfs_file_flush(lfs_t *lfs, lfs_file_t *file) {
             .pos = file->pos,
             .cache = lfs->rcache,
         };
-        lfs->rcache.block = 0xffffffff;
+        lfs_cache_drop(lfs, &lfs->rcache);
 
         while (file->pos < file->size) {
             // copy over a byte at a time, leave it up to caching
@@ -1451,8 +1480,8 @@ static int lfs_file_flush(lfs_t *lfs, lfs_file_t *file) {
 
             // keep our reference to the rcache in sync
             if (lfs->rcache.block != 0xffffffff) {
-                orig.cache.block = 0xffffffff;
-                lfs->rcache.block = 0xffffffff;
+                lfs_cache_drop(lfs, &orig.cache);
+                lfs_cache_drop(lfs, &lfs->rcache);
             }
         }
 
@@ -1630,7 +1659,7 @@ lfs_ssize_t lfs_file_write(lfs_t *lfs, lfs_file_t *file,
                 }
 
                 // mark cache as dirty since we may have read data into it
-                file->cache.block = 0xffffffff;
+                lfs_cache_zero(lfs, &file->cache);
             }
 
             // extend file with new blocks
@@ -1981,7 +2010,6 @@ static int lfs_init(lfs_t *lfs, const struct lfs_config *cfg) {
     lfs->cfg = cfg;
 
     // setup read cache
-    lfs->rcache.block = 0xffffffff;
     if (lfs->cfg->read_buffer) {
         lfs->rcache.buffer = lfs->cfg->read_buffer;
     } else {
@@ -1992,7 +2020,6 @@ static int lfs_init(lfs_t *lfs, const struct lfs_config *cfg) {
     }
 
     // setup program cache
-    lfs->pcache.block = 0xffffffff;
     if (lfs->cfg->prog_buffer) {
         lfs->pcache.buffer = lfs->cfg->prog_buffer;
     } else {
@@ -2002,6 +2029,10 @@ static int lfs_init(lfs_t *lfs, const struct lfs_config *cfg) {
         }
     }
 
+    // zero to avoid information leaks
+    lfs_cache_zero(lfs, &lfs->rcache);
+    lfs_cache_zero(lfs, &lfs->pcache);
+
     // setup lookahead, round down to nearest 32-bits
     LFS_ASSERT(lfs->cfg->lookahead % 32 == 0);
     LFS_ASSERT(lfs->cfg->lookahead > 0);
@@ -2378,7 +2409,8 @@ static int lfs_relocate(lfs_t *lfs,
 
         // update internal root
         if (lfs_paircmp(oldpair, lfs->root) == 0) {
-            LFS_DEBUG("Relocating root %d %d", newpair[0], newpair[1]);
+            LFS_DEBUG("Relocating root %" PRIu32 " %" PRIu32,
+                    newpair[0], newpair[1]);
             lfs->root[0] = newpair[0];
             lfs->root[1] = newpair[1];
         }
@@ -2434,7 +2466,7 @@ int lfs_deorphan(lfs_t *lfs) {
 
             if (!res) {
                 // we are an orphan
-                LFS_DEBUG("Found orphan %d %d",
+                LFS_DEBUG("Found orphan %" PRIu32 " %" PRIu32,
                         pdir.d.tail[0], pdir.d.tail[1]);
 
                 pdir.d.tail[0] = cwd.d.tail[0];
@@ -2450,7 +2482,7 @@ int lfs_deorphan(lfs_t *lfs) {
 
             if (!lfs_pairsync(entry.d.u.dir, pdir.d.tail)) {
                 // we have desynced
-                LFS_DEBUG("Found desync %d %d",
+                LFS_DEBUG("Found desync %" PRIu32 " %" PRIu32,
                         entry.d.u.dir[0], entry.d.u.dir[1]);
 
                 pdir.d.tail[0] = entry.d.u.dir[0];
@@ -2485,14 +2517,14 @@ int lfs_deorphan(lfs_t *lfs) {
                 }
 
                 if (moved) {
-                    LFS_DEBUG("Found move %d %d",
+                    LFS_DEBUG("Found move %" PRIu32 " %" PRIu32,
                             entry.d.u.dir[0], entry.d.u.dir[1]);
                     err = lfs_dir_remove(lfs, &cwd, &entry);
                     if (err) {
                         return err;
                     }
                 } else {
-                    LFS_DEBUG("Found partial move %d %d",
+                    LFS_DEBUG("Found partial move %" PRIu32 " %" PRIu32,
                             entry.d.u.dir[0], entry.d.u.dir[1]);
                     entry.d.type &= ~0x80;
                     err = lfs_dir_update(lfs, &cwd, &entry, NULL);
@@ -2508,4 +2540,3 @@ int lfs_deorphan(lfs_t *lfs) {
 
     return 0;
 }
-

lfs.h

@@ -10,6 +10,11 @@
 #include <stdint.h>
 #include <stdbool.h>
 
+#ifdef __cplusplus
+extern "C"
+{
+#endif
+
 
 /// Version info ///
 
@@ -460,4 +465,8 @@ int lfs_traverse(lfs_t *lfs, int (*cb)(void*, lfs_block_t), void *data);
 int lfs_deorphan(lfs_t *lfs);
 
 
+#ifdef __cplusplus
+} /* extern "C" */
+#endif
+
 #endif

lfs_util.h

@@ -34,6 +34,11 @@
 #include <stdio.h>
 #endif
 
+#ifdef __cplusplus
+extern "C"
+{
+#endif
+
 
 // Macros, may be replaced by system specific wrappers. Arguments to these
 // macros must not have side-effects as the macros can be removed for a smaller
@@ -158,6 +163,7 @@ static inline void *lfs_malloc(size_t size) {
 #ifndef LFS_NO_MALLOC
     return malloc(size);
 #else
+    (void)size;
     return NULL;
 #endif
 }
@@ -166,9 +172,15 @@ static inline void *lfs_malloc(size_t size) {
 static inline void lfs_free(void *p) {
 #ifndef LFS_NO_MALLOC
     free(p);
+#else
+    (void)p;
 #endif
 }
 
 
+#ifdef __cplusplus
+} /* extern "C" */
+#endif
+
 #endif
 #endif

tests/test_files.sh

@@ -30,7 +30,7 @@ TEST
 
 w_test() {
 tests/test.py << TEST
-    lfs_size_t size = $1;
+    size = $1;
     lfs_size_t chunk = 31;
     srand(0);
     lfs_mount(&lfs, &cfg) => 0;
@@ -50,7 +50,7 @@ TEST
 
 r_test() {
 tests/test.py << TEST
-    lfs_size_t size = $1;
+    size = $1;
     lfs_size_t chunk = 29;
     srand(0);
     lfs_mount(&lfs, &cfg) => 0;

tests/test_seek.sh

@@ -153,7 +153,7 @@ tests/test.py << TEST
     lfs_file_read(&lfs, &file[0], buffer, size) => size;
     memcmp(buffer, "kittycatcat", size) => 0;
 
-    lfs_size_t size = lfs_file_size(&lfs, &file[0]);
+    size = lfs_file_size(&lfs, &file[0]);
     lfs_file_seek(&lfs, &file[0], 0, LFS_SEEK_CUR) => size;
 
     lfs_file_close(&lfs, &file[0]) => 0;
@@ -202,7 +202,7 @@ tests/test.py << TEST
     lfs_file_read(&lfs, &file[0], buffer, size) => size;
     memcmp(buffer, "kittycatcat", size) => 0;
 
-    lfs_size_t size = lfs_file_size(&lfs, &file[0]);
+    size = lfs_file_size(&lfs, &file[0]);
     lfs_file_seek(&lfs, &file[0], 0, LFS_SEEK_CUR) => size;
 
     lfs_file_close(&lfs, &file[0]) => 0;
@@ -243,7 +243,7 @@ tests/test.py << TEST
     lfs_file_read(&lfs, &file[0], buffer, size) => size;
     memcmp(buffer, "kittycatcat", size) => 0;
 
-    lfs_size_t size = lfs_file_size(&lfs, &file[0]);
+    size = lfs_file_size(&lfs, &file[0]);
     lfs_file_seek(&lfs, &file[0], 0, LFS_SEEK_CUR) => size;
 
     lfs_file_close(&lfs, &file[0]) => 0;
@@ -286,7 +286,7 @@ tests/test.py << TEST
     lfs_file_read(&lfs, &file[0], buffer, size) => size;
     memcmp(buffer, "kittycatcat", size) => 0;
 
-    lfs_size_t size = lfs_file_size(&lfs, &file[0]);
+    size = lfs_file_size(&lfs, &file[0]);
     lfs_file_seek(&lfs, &file[0], 0, LFS_SEEK_CUR) => size;
 
     lfs_file_close(&lfs, &file[0]) => 0;