Merge branch 'devel' into ci-revamp

Needed to bring in new "error-asserts" configuration

.github/workflows/release.yml

@@ -96,7 +96,7 @@ jobs:
                   | capture("Code size is (?<result>[0-9]+)").result' \
                 prev-results.json || echo 0)"
             ./scripts/code.py -u results/code-thumb-readonly.csv -s | awk '
-              NR==2 {printf "Code size (readonly),%d B",$2}
+              NR==2 {printf "Code size<br/>(readonly),%d B",$2}
               NR==2 && ENVIRON["PREV"]+0 != 0 {
                 printf " (%+.1f%%)",100*($2-ENVIRON["PREV"])/ENVIRON["PREV"]}
               NR==2 {printf "\n"}' \
@@ -107,7 +107,7 @@ jobs:
                   | capture("Code size is (?<result>[0-9]+)").result' \
                 prev-results.json || echo 0)"
             ./scripts/code.py -u results/code-thumb-threadsafe.csv -s | awk '
-              NR==2 {printf "Code size (threadsafe),%d B",$2}
+              NR==2 {printf "Code size<br/>(threadsafe),%d B",$2}
               NR==2 && ENVIRON["PREV"]+0 != 0 {
                 printf " (%+.1f%%)",100*($2-ENVIRON["PREV"])/ENVIRON["PREV"]}
               NR==2 {printf "\n"}' \
@@ -118,7 +118,18 @@ jobs:
                   | capture("Code size is (?<result>[0-9]+)").result' \
                 prev-results.json || echo 0)"
             ./scripts/code.py -u results/code-thumb-migrate.csv -s | awk '
-              NR==2 {printf "Code size (migrate),%d B",$2}
+              NR==2 {printf "Code size<br/>(migrate),%d B",$2}
+              NR==2 && ENVIRON["PREV"]+0 != 0 {
+                printf " (%+.1f%%)",100*($2-ENVIRON["PREV"])/ENVIRON["PREV"]}
+              NR==2 {printf "\n"}' \
+            >> results.csv)
+          [ -e results/code-thumb-error-asserts.csv ] && ( \
+            export PREV="$(jq -re '
+                  select(.context == "results / code (error-asserts)").description
+                  | capture("Code size is (?<result>[0-9]+)").result' \
+                prev-results.json || echo 0)"
+            ./scripts/code.py -u results/code-thumb-error-asserts.csv -s | awk '
+              NR==2 {printf "Code size<br/>(error-asserts),%d B",$2}
               NR==2 && ENVIRON["PREV"]+0 != 0 {
                 printf " (%+.1f%%)",100*($2-ENVIRON["PREV"])/ENVIRON["PREV"]}
               NR==2 {printf "\n"}' \

.github/workflows/test.yml

@@ -195,6 +195,17 @@ jobs:
               -DLFS_NO_ERROR \
               -DLFS_MIGRATE" \
             CODEFLAGS+="-o results/code-${{matrix.arch}}-migrate.csv"
+      - name: results-code-error-asserts
+        run: |
+          mkdir -p results
+          make clean
+          make code \
+            CFLAGS+=" \
+              -DLFS_NO_DEBUG \
+              -DLFS_NO_WARN \
+              -DLFS_NO_ERROR \
+              -D'LFS_ASSERT(test)=do {if(!(test)) {return -1;}} while(0)'" \
+            CODEFLAGS+="-o results/code-${{matrix.arch}}-error-asserts.csv"
       - name: upload-results
         uses: actions/upload-artifact@v2
         with:

lfs.c

@@ -425,7 +425,8 @@ static inline void lfs_superblock_tole32(lfs_superblock_t *superblock) {
     superblock->attr_max    = lfs_tole32(superblock->attr_max);
 }
 
-static inline bool lfs_mlist_isopen(struct lfs_mlist *head,
+#ifndef LFS_NO_ASSERT
+static bool lfs_mlist_isopen(struct lfs_mlist *head,
         struct lfs_mlist *node) {
     for (struct lfs_mlist **p = &head; *p; p = &(*p)->next) {
         if (*p == (struct lfs_mlist*)node) {
@@ -435,8 +436,9 @@ static inline bool lfs_mlist_isopen(struct lfs_mlist *head,
 
     return false;
 }
+#endif
 
-static inline void lfs_mlist_remove(lfs_t *lfs, struct lfs_mlist *mlist) {
+static void lfs_mlist_remove(lfs_t *lfs, struct lfs_mlist *mlist) {
     for (struct lfs_mlist **p = &lfs->mlist; *p; p = &(*p)->next) {
         if (*p == mlist) {
             *p = (*p)->next;
@@ -445,7 +447,7 @@ static inline void lfs_mlist_remove(lfs_t *lfs, struct lfs_mlist *mlist) {
     }
 }
 
-static inline void lfs_mlist_append(lfs_t *lfs, struct lfs_mlist *mlist) {
+static void lfs_mlist_append(lfs_t *lfs, struct lfs_mlist *mlist) {
     mlist->next = lfs->mlist;
     lfs->mlist = mlist;
 }
@@ -465,7 +467,7 @@ static int lfs_file_rawsync(lfs_t *lfs, lfs_file_t *file);
 static int lfs_file_outline(lfs_t *lfs, lfs_file_t *file);
 static int lfs_file_flush(lfs_t *lfs, lfs_file_t *file);
 
-static void lfs_fs_preporphans(lfs_t *lfs, int8_t orphans);
+static int lfs_fs_preporphans(lfs_t *lfs, int8_t orphans);
 static void lfs_fs_prepmove(lfs_t *lfs,
         uint16_t id, const lfs_block_t pair[2]);
 static int lfs_fs_pred(lfs_t *lfs, const lfs_block_t dir[2],
@@ -1589,7 +1591,8 @@ static int lfs_dir_compact(lfs_t *lfs,
         // for metadata updates.
         if (end - begin < 0xff &&
                 size <= lfs_min(lfs->cfg->block_size - 36,
-                    lfs_alignup(lfs->cfg->block_size/2,
+                    lfs_alignup((lfs->cfg->metadata_max ?
+                            lfs->cfg->metadata_max : lfs->cfg->block_size)/2,
                         lfs->cfg->prog_size))) {
             break;
         }
@@ -1674,7 +1677,8 @@ static int lfs_dir_compact(lfs_t *lfs,
                 .crc = 0xffffffff,
 
                 .begin = 0,
-                .end = lfs->cfg->block_size - 8,
+                .end = (lfs->cfg->metadata_max ?
+                    lfs->cfg->metadata_max : lfs->cfg->block_size) - 8,
             };
 
             // erase block to write to
@@ -1884,7 +1888,8 @@ static int lfs_dir_commit(lfs_t *lfs, lfs_mdir_t *dir,
             .crc = 0xffffffff,
 
             .begin = dir->off,
-            .end = lfs->cfg->block_size - 8,
+            .end = (lfs->cfg->metadata_max ?
+                lfs->cfg->metadata_max : lfs->cfg->block_size) - 8,
         };
 
         // traverse attrs that need to be written out
@@ -2061,7 +2066,10 @@ static int lfs_rawmkdir(lfs_t *lfs, const char *path) {
     // current block end of list?
     if (cwd.m.split) {
         // update tails, this creates a desync
-        lfs_fs_preporphans(lfs, +1);
+        err = lfs_fs_preporphans(lfs, +1);
+        if (err) {
+            return err;
+        }
 
         // it's possible our predecessor has to be relocated, and if
         // our parent is our predecessor's predecessor, this could have
@@ -2081,7 +2089,10 @@ static int lfs_rawmkdir(lfs_t *lfs, const char *path) {
         }
 
         lfs->mlist = cwd.next;
-        lfs_fs_preporphans(lfs, -1);
+        err = lfs_fs_preporphans(lfs, -1);
+        if (err) {
+            return err;
+        }
     }
 
     // now insert into our parent block
@@ -2966,7 +2977,9 @@ static lfs_ssize_t lfs_file_rawwrite(lfs_t *lfs, lfs_file_t *file,
     if ((file->flags & LFS_F_INLINE) &&
             lfs_max(file->pos+nsize, file->ctz.size) >
             lfs_min(0x3fe, lfs_min(
-                lfs->cfg->cache_size, lfs->cfg->block_size/8))) {
+                lfs->cfg->cache_size,
+                (lfs->cfg->metadata_max ?
+                    lfs->cfg->metadata_max : lfs->cfg->block_size) / 8))) {
         // inline file doesn't fit anymore
         int err = lfs_file_outline(lfs, file);
         if (err) {
@@ -3048,14 +3061,6 @@ relocate:
 
 static lfs_soff_t lfs_file_rawseek(lfs_t *lfs, lfs_file_t *file,
         lfs_soff_t off, int whence) {
-#ifndef LFS_READONLY
-    // write out everything beforehand, may be noop if rdonly
-    int err = lfs_file_flush(lfs, file);
-    if (err) {
-        return err;
-    }
-#endif
-
     // find new pos
     lfs_off_t npos = file->pos;
     if (whence == LFS_SEEK_SET) {
@@ -3063,7 +3068,7 @@ static lfs_soff_t lfs_file_rawseek(lfs_t *lfs, lfs_file_t *file,
     } else if (whence == LFS_SEEK_CUR) {
         npos = file->pos + off;
     } else if (whence == LFS_SEEK_END) {
-        npos = file->ctz.size + off;
+        npos = lfs_file_rawsize(lfs, file) + off;
     }
 
     if (npos > lfs->file_max) {
@@ -3071,6 +3076,19 @@ static lfs_soff_t lfs_file_rawseek(lfs_t *lfs, lfs_file_t *file,
         return LFS_ERR_INVAL;
     }
 
+    if (file->pos == npos) {
+        // noop - position has not changed
+        return npos;
+    }
+
+#ifndef LFS_READONLY
+    // write out everything beforehand, may be noop if rdonly
+    int err = lfs_file_flush(lfs, file);
+    if (err) {
+        return err;
+    }
+#endif
+
     // update pos
     file->pos = npos;
     return npos;
@@ -3101,21 +3119,22 @@ static int lfs_file_rawtruncate(lfs_t *lfs, lfs_file_t *file, lfs_off_t size) {
             return err;
         }
 
+        // need to set pos/block/off consistently so seeking back to
+        // the old position does not get confused
+        file->pos = size;
         file->ctz.head = file->block;
         file->ctz.size = size;
         file->flags |= LFS_F_DIRTY | LFS_F_READING;
     } else if (size > oldsize) {
         // flush+seek if not already at end
-        if (file->pos != oldsize) {
-            lfs_soff_t res = lfs_file_rawseek(lfs, file, 0, LFS_SEEK_END);
-            if (res < 0) {
-                return (int)res;
-            }
+        lfs_soff_t res = lfs_file_rawseek(lfs, file, 0, LFS_SEEK_END);
+        if (res < 0) {
+            return (int)res;
         }
 
         // fill with zeros
         while (file->pos < size) {
-            lfs_ssize_t res = lfs_file_rawwrite(lfs, file, &(uint8_t){0}, 1);
+            res = lfs_file_rawwrite(lfs, file, &(uint8_t){0}, 1);
             if (res < 0) {
                 return (int)res;
             }
@@ -3206,7 +3225,10 @@ static int lfs_rawremove(lfs_t *lfs, const char *path) {
         }
 
         // mark fs as orphaned
-        lfs_fs_preporphans(lfs, +1);
+        err = lfs_fs_preporphans(lfs, +1);
+        if (err) {
+            return err;
+        }
 
         // I know it's crazy but yes, dir can be changed by our parent's
         // commit (if predecessor is child)
@@ -3226,7 +3248,10 @@ static int lfs_rawremove(lfs_t *lfs, const char *path) {
     lfs->mlist = dir.next;
     if (lfs_tag_type3(tag) == LFS_TYPE_DIR) {
         // fix orphan
-        lfs_fs_preporphans(lfs, -1);
+        err = lfs_fs_preporphans(lfs, -1);
+        if (err) {
+            return err;
+        }
 
         err = lfs_fs_pred(lfs, dir.m.pair, &cwd);
         if (err) {
@@ -3312,7 +3337,10 @@ static int lfs_rawrename(lfs_t *lfs, const char *oldpath, const char *newpath) {
         }
 
         // mark fs as orphaned
-        lfs_fs_preporphans(lfs, +1);
+        err = lfs_fs_preporphans(lfs, +1);
+        if (err) {
+            return err;
+        }
 
         // I know it's crazy but yes, dir can be changed by our parent's
         // commit (if predecessor is child)
@@ -3355,7 +3383,10 @@ static int lfs_rawrename(lfs_t *lfs, const char *oldpath, const char *newpath) {
     lfs->mlist = prevdir.next;
     if (prevtag != LFS_ERR_NOENT && lfs_tag_type3(prevtag) == LFS_TYPE_DIR) {
         // fix orphan
-        lfs_fs_preporphans(lfs, -1);
+        err = lfs_fs_preporphans(lfs, -1);
+        if (err) {
+            return err;
+        }
 
         err = lfs_fs_pred(lfs, prevdir.m.pair, &newcwd);
         if (err) {
@@ -3536,6 +3567,8 @@ static int lfs_init(lfs_t *lfs, const struct lfs_config *cfg) {
         lfs->attr_max = LFS_ATTR_MAX;
     }
 
+    LFS_ASSERT(lfs->cfg->metadata_max <= lfs->cfg->block_size);
+
     // setup default state
     lfs->root[0] = LFS_BLOCK_NULL;
     lfs->root[1] = LFS_BLOCK_NULL;
@@ -3616,12 +3649,6 @@ static int lfs_rawformat(lfs_t *lfs, const struct lfs_config *cfg) {
             goto cleanup;
         }
 
-        // sanity check that fetch works
-        err = lfs_dir_fetch(lfs, &root, (const lfs_block_t[2]){0, 1});
-        if (err) {
-            goto cleanup;
-        }
-
         // force compaction to prevent accidentally mounting any
         // older version of littlefs that may live on disk
         root.erased = false;
@@ -3629,6 +3656,12 @@ static int lfs_rawformat(lfs_t *lfs, const struct lfs_config *cfg) {
         if (err) {
             goto cleanup;
         }
+
+        // sanity check that fetch works
+        err = lfs_dir_fetch(lfs, &root, (const lfs_block_t[2]){0, 1});
+        if (err) {
+            goto cleanup;
+        }
     }
 
 cleanup:
@@ -3829,7 +3862,7 @@ int lfs_fs_rawtraverse(lfs_t *lfs,
                 if (err) {
                     return err;
                 }
-            } else if (includeorphans && 
+            } else if (includeorphans &&
                     lfs_tag_type3(tag) == LFS_TYPE_DIRSTRUCT) {
                 for (int i = 0; i < 2; i++) {
                     err = cb(data, (&ctz.head)[i]);
@@ -3986,7 +4019,10 @@ static int lfs_fs_relocate(lfs_t *lfs,
 
     if (tag != LFS_ERR_NOENT) {
         // update disk, this creates a desync
-        lfs_fs_preporphans(lfs, +1);
+        int err = lfs_fs_preporphans(lfs, +1);
+        if (err) {
+            return err;
+        }
 
         // fix pending move in this pair? this looks like an optimization but
         // is in fact _required_ since relocating may outdate the move.
@@ -4003,7 +4039,7 @@ static int lfs_fs_relocate(lfs_t *lfs,
         }
 
         lfs_pair_tole32(newpair);
-        int err = lfs_dir_commit(lfs, &parent, LFS_MKATTRS(
+        err = lfs_dir_commit(lfs, &parent, LFS_MKATTRS(
                 {LFS_MKTAG_IF(moveid != 0x3ff,
                     LFS_TYPE_DELETE, moveid, 0), NULL},
                 {tag, newpair}));
@@ -4013,7 +4049,10 @@ static int lfs_fs_relocate(lfs_t *lfs,
         }
 
         // next step, clean up orphans
-        lfs_fs_preporphans(lfs, -1);
+        err = lfs_fs_preporphans(lfs, -1);
+        if (err) {
+            return err;
+        }
     }
 
     // find pred
@@ -4052,11 +4091,13 @@ static int lfs_fs_relocate(lfs_t *lfs,
 #endif
 
 #ifndef LFS_READONLY
-static void lfs_fs_preporphans(lfs_t *lfs, int8_t orphans) {
+static int lfs_fs_preporphans(lfs_t *lfs, int8_t orphans) {
     LFS_ASSERT(lfs_tag_size(lfs->gstate.tag) > 0 || orphans >= 0);
     lfs->gstate.tag += orphans;
     lfs->gstate.tag = ((lfs->gstate.tag & ~LFS_MKTAG(0x800, 0, 0)) |
             ((uint32_t)lfs_gstate_hasorphans(&lfs->gstate) << 31));
+
+    return 0;
 }
 #endif
 
@@ -4173,8 +4214,7 @@ static int lfs_fs_deorphan(lfs_t *lfs) {
     }
 
     // mark orphans as fixed
-    lfs_fs_preporphans(lfs, -lfs_gstate_getorphans(&lfs->gstate));
-    return 0;
+    return lfs_fs_preporphans(lfs, -lfs_gstate_getorphans(&lfs->gstate));
 }
 #endif
 

lfs.h

@@ -207,7 +207,7 @@ struct lfs_config {
     // Number of erasable blocks on the device.
     lfs_size_t block_count;
 
-    // Number of erase cycles before littlefs evicts metadata logs and moves 
+    // Number of erase cycles before littlefs evicts metadata logs and moves
     // the metadata to another block. Suggested values are in the
     // range 100-1000, with large values having better performance at the cost
     // of less consistent wear distribution.
@@ -256,6 +256,12 @@ struct lfs_config {
     // larger attributes size but must be <= LFS_ATTR_MAX. Defaults to
     // LFS_ATTR_MAX when zero.
     lfs_size_t attr_max;
+
+    // Optional upper limit on total space given to metadata pairs in bytes. On
+    // devices with large blocks (e.g. 128kB) setting this to a low size (2-8kB)
+    // can help bound the metadata compaction time. Must be <= block_size.
+    // Defaults to block_size when zero.
+    lfs_size_t metadata_max;
 };
 
 // File info structure

scripts/test.py

@@ -812,9 +812,9 @@ if __name__ == "__main__":
     parser.add_argument('test_paths', nargs='*', default=[TEST_PATHS],
         help="Description of test(s) to run. By default, this is all tests \
             found in the \"{0}\" directory. Here, you can specify a different \
-            directory of tests, a specific file, a suite by name, and even a \
-            specific test case by adding brackets. For example \
-            \"test_dirs[0]\" or \"{0}/test_dirs.toml[0]\".".format(TEST_PATHS))
+            directory of tests, a specific file, a suite by name, and even \
+            specific test cases and permutations. For example \
+            \"test_dirs#1\" or \"{0}/test_dirs.toml#1#1\".".format(TEST_PATHS))
     parser.add_argument('-D', action='append', default=[],
         help="Overriding parameter definitions.")
     parser.add_argument('-v', '--verbose', action='store_true',

tests/test_truncate.toml

@@ -392,3 +392,48 @@ code = '''
 
     lfs_unmount(&lfs) => 0;
 '''
+
+[[case]] # noop truncate
+define.MEDIUMSIZE = [32, 2048]
+code = '''
+    lfs_format(&lfs, &cfg) => 0;
+    lfs_mount(&lfs, &cfg) => 0;
+    lfs_file_open(&lfs, &file, "baldynoop",
+            LFS_O_RDWR | LFS_O_CREAT) => 0;
+
+    strcpy((char*)buffer, "hair");
+    size = strlen((char*)buffer);
+    for (lfs_off_t j = 0; j < MEDIUMSIZE; j += size) {
+        lfs_file_write(&lfs, &file, buffer, size) => size;
+
+        // this truncate should do nothing
+        lfs_file_truncate(&lfs, &file, j+size) => 0;
+    }
+    lfs_file_size(&lfs, &file) => MEDIUMSIZE;
+
+    lfs_file_seek(&lfs, &file, 0, LFS_SEEK_SET) => 0;
+    // should do nothing again
+    lfs_file_truncate(&lfs, &file, MEDIUMSIZE) => 0;
+    lfs_file_size(&lfs, &file) => MEDIUMSIZE;
+
+    for (lfs_off_t j = 0; j < MEDIUMSIZE; j += size) {
+        lfs_file_read(&lfs, &file, buffer, size) => size;
+        memcmp(buffer, "hair", size) => 0;
+    }
+    lfs_file_read(&lfs, &file, buffer, size) => 0;
+
+    lfs_file_close(&lfs, &file) => 0;
+    lfs_unmount(&lfs) => 0;
+
+    // still there after reboot?
+    lfs_mount(&lfs, &cfg) => 0;
+    lfs_file_open(&lfs, &file, "baldynoop", LFS_O_RDWR) => 0;
+    lfs_file_size(&lfs, &file) => MEDIUMSIZE;
+    for (lfs_off_t j = 0; j < MEDIUMSIZE; j += size) {
+        lfs_file_read(&lfs, &file, buffer, size) => size;
+        memcmp(buffer, "hair", size) => 0;
+    }
+    lfs_file_read(&lfs, &file, buffer, size) => 0;
+    lfs_file_close(&lfs, &file) => 0;
+    lfs_unmount(&lfs) => 0;
+'''