From 749a45650fc2d723b3cfe1748266c95105323987 Mon Sep 17 00:00:00 2001 From: Brian Pugh Date: Tue, 16 Apr 2024 20:32:12 -0700 Subject: [PATCH 1/3] Fix DivideByZero exception when filesystem is completely full. --- lfs.c | 2 +- lfs.h | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/lfs.c b/lfs.c index a0bd76fd..c14efaac 100644 --- a/lfs.c +++ b/lfs.c @@ -688,7 +688,7 @@ static int lfs_alloc(lfs_t *lfs, lfs_block_t *block) { if (lfs->lookahead.ckpoint <= 0) { LFS_ERROR("No more free space 0x%"PRIx32, (lfs->lookahead.start + lfs->lookahead.next) - % lfs->cfg->block_count); + % lfs->block_count); return LFS_ERR_NOSPC; } diff --git a/lfs.h b/lfs.h index 99145022..274259e2 100644 --- a/lfs.h +++ b/lfs.h @@ -204,6 +204,7 @@ struct lfs_config { lfs_size_t block_size; // Number of erasable blocks on the device. + // If 0, will attempt to infer block_count from existing filesystem. lfs_size_t block_count; // Number of erase cycles before littlefs evicts metadata logs and moves From 01b6a47ea87b7f5afc8951110c1fe9a90e8ceafc Mon Sep 17 00:00:00 2001 From: Christopher Haster Date: Wed, 17 Apr 2024 00:02:04 -0500 Subject: [PATCH 2/3] Extended test_alloc to test inferred block_count The block allocator is an area where inferred block counts (when cfg.block_count=0) are more likely to cause problems. As is shown by the recent divide-by-zero-exhaustion issue. --- tests/test_alloc.toml | 69 ++++++++++++++++++++++++++++++++----------- 1 file changed, 52 insertions(+), 17 deletions(-) diff --git a/tests/test_alloc.toml b/tests/test_alloc.toml index 9e4daee5..338c75d5 100644 --- a/tests/test_alloc.toml +++ b/tests/test_alloc.toml @@ -8,17 +8,22 @@ defines.FILES = 3 defines.SIZE = '(((BLOCK_SIZE-8)*(BLOCK_COUNT-6)) / FILES)' defines.GC = [false, true] defines.COMPACT_THRESH = ['-1', '0', 'BLOCK_SIZE/2'] +defines.INFER_BC = [false, true] code = ''' const char *names[] = {"bacon", "eggs", "pancakes"}; lfs_file_t files[FILES]; lfs_t lfs; lfs_format(&lfs, cfg) => 0; - lfs_mount(&lfs, cfg) => 0; + struct lfs_config cfg_ = *cfg; + if (INFER_BC) { + cfg_.block_count = 0; + } + lfs_mount(&lfs, &cfg_) => 0; lfs_mkdir(&lfs, "breakfast") => 0; lfs_unmount(&lfs) => 0; - lfs_mount(&lfs, cfg) => 0; + lfs_mount(&lfs, &cfg_) => 0; for (int n = 0; n < FILES; n++) { char path[1024]; sprintf(path, "breakfast/%s", names[n]); @@ -39,7 +44,7 @@ code = ''' } lfs_unmount(&lfs) => 0; - lfs_mount(&lfs, cfg) => 0; + lfs_mount(&lfs, &cfg_) => 0; for (int n = 0; n < FILES; n++) { char path[1024]; sprintf(path, "breakfast/%s", names[n]); @@ -62,17 +67,22 @@ defines.FILES = 3 defines.SIZE = '(((BLOCK_SIZE-8)*(BLOCK_COUNT-6)) / FILES)' defines.GC = [false, true] defines.COMPACT_THRESH = ['-1', '0', 'BLOCK_SIZE/2'] +defines.INFER_BC = [false, true] code = ''' const char *names[] = {"bacon", "eggs", "pancakes"}; lfs_t lfs; lfs_format(&lfs, cfg) => 0; - lfs_mount(&lfs, cfg) => 0; + struct lfs_config cfg_ = *cfg; + if (INFER_BC) { + cfg_.block_count = 0; + } + lfs_mount(&lfs, &cfg_) => 0; lfs_mkdir(&lfs, "breakfast") => 0; lfs_unmount(&lfs) => 0; for (int n = 0; n < FILES; n++) { - lfs_mount(&lfs, cfg) => 0; + lfs_mount(&lfs, &cfg_) => 0; char path[1024]; sprintf(path, "breakfast/%s", names[n]); lfs_file_t file; @@ -91,7 +101,7 @@ code = ''' lfs_unmount(&lfs) => 0; } - lfs_mount(&lfs, cfg) => 0; + lfs_mount(&lfs, &cfg_) => 0; for (int n = 0; n < FILES; n++) { char path[1024]; sprintf(path, "breakfast/%s", names[n]); @@ -113,19 +123,24 @@ code = ''' defines.FILES = 3 defines.SIZE = '(((BLOCK_SIZE-8)*(BLOCK_COUNT-6)) / FILES)' defines.CYCLES = [1, 10] +defines.INFER_BC = [false, true] code = ''' const char *names[] = {"bacon", "eggs", "pancakes"}; lfs_file_t files[FILES]; lfs_t lfs; lfs_format(&lfs, cfg) => 0; + struct lfs_config cfg_ = *cfg; + if (INFER_BC) { + cfg_.block_count = 0; + } for (int c = 0; c < CYCLES; c++) { - lfs_mount(&lfs, cfg) => 0; + lfs_mount(&lfs, &cfg_) => 0; lfs_mkdir(&lfs, "breakfast") => 0; lfs_unmount(&lfs) => 0; - lfs_mount(&lfs, cfg) => 0; + lfs_mount(&lfs, &cfg_) => 0; for (int n = 0; n < FILES; n++) { char path[1024]; sprintf(path, "breakfast/%s", names[n]); @@ -143,7 +158,7 @@ code = ''' } lfs_unmount(&lfs) => 0; - lfs_mount(&lfs, cfg) => 0; + lfs_mount(&lfs, &cfg_) => 0; for (int n = 0; n < FILES; n++) { char path[1024]; sprintf(path, "breakfast/%s", names[n]); @@ -159,7 +174,7 @@ code = ''' } lfs_unmount(&lfs) => 0; - lfs_mount(&lfs, cfg) => 0; + lfs_mount(&lfs, &cfg_) => 0; for (int n = 0; n < FILES; n++) { char path[1024]; sprintf(path, "breakfast/%s", names[n]); @@ -175,19 +190,24 @@ code = ''' defines.FILES = 3 defines.SIZE = '(((BLOCK_SIZE-8)*(BLOCK_COUNT-6)) / FILES)' defines.CYCLES = [1, 10] +defines.INFER_BC = [false, true] code = ''' const char *names[] = {"bacon", "eggs", "pancakes"}; lfs_t lfs; lfs_format(&lfs, cfg) => 0; + struct lfs_config cfg_ = *cfg; + if (INFER_BC) { + cfg_.block_count = 0; + } for (int c = 0; c < CYCLES; c++) { - lfs_mount(&lfs, cfg) => 0; + lfs_mount(&lfs, &cfg_) => 0; lfs_mkdir(&lfs, "breakfast") => 0; lfs_unmount(&lfs) => 0; for (int n = 0; n < FILES; n++) { - lfs_mount(&lfs, cfg) => 0; + lfs_mount(&lfs, &cfg_) => 0; char path[1024]; sprintf(path, "breakfast/%s", names[n]); lfs_file_t file; @@ -232,10 +252,15 @@ code = ''' # exhaustion test [cases.test_alloc_exhaustion] +defines.INFER_BC = [false, true] code = ''' lfs_t lfs; lfs_format(&lfs, cfg) => 0; - lfs_mount(&lfs, cfg) => 0; + struct lfs_config cfg_ = *cfg; + if (INFER_BC) { + cfg_.block_count = 0; + } + lfs_mount(&lfs, &cfg_) => 0; lfs_file_t file; lfs_file_open(&lfs, &file, "exhaustion", LFS_O_WRONLY | LFS_O_CREAT); size_t size = strlen("exhaustion"); @@ -263,7 +288,7 @@ code = ''' lfs_file_close(&lfs, &file) => 0; lfs_unmount(&lfs) => 0; - lfs_mount(&lfs, cfg) => 0; + lfs_mount(&lfs, &cfg_) => 0; lfs_file_open(&lfs, &file, "exhaustion", LFS_O_RDONLY); size = strlen("exhaustion"); lfs_file_size(&lfs, &file) => size; @@ -276,10 +301,15 @@ code = ''' # exhaustion wraparound test [cases.test_alloc_exhaustion_wraparound] defines.SIZE = '(((BLOCK_SIZE-8)*(BLOCK_COUNT-4)) / 3)' +defines.INFER_BC = [false, true] code = ''' lfs_t lfs; lfs_format(&lfs, cfg) => 0; - lfs_mount(&lfs, cfg) => 0; + struct lfs_config cfg_ = *cfg; + if (INFER_BC) { + cfg_.block_count = 0; + } + lfs_mount(&lfs, &cfg_) => 0; lfs_file_t file; lfs_file_open(&lfs, &file, "padding", LFS_O_WRONLY | LFS_O_CREAT); @@ -317,7 +347,7 @@ code = ''' lfs_file_close(&lfs, &file) => 0; lfs_unmount(&lfs) => 0; - lfs_mount(&lfs, cfg) => 0; + lfs_mount(&lfs, &cfg_) => 0; lfs_file_open(&lfs, &file, "exhaustion", LFS_O_RDONLY); size = strlen("exhaustion"); lfs_file_size(&lfs, &file) => size; @@ -330,10 +360,15 @@ code = ''' # dir exhaustion test [cases.test_alloc_dir_exhaustion] +defines.INFER_BC = [false, true] code = ''' lfs_t lfs; lfs_format(&lfs, cfg) => 0; - lfs_mount(&lfs, cfg) => 0; + struct lfs_config cfg_ = *cfg; + if (INFER_BC) { + cfg_.block_count = 0; + } + lfs_mount(&lfs, &cfg_) => 0; // find out max file size lfs_mkdir(&lfs, "exhaustiondir") => 0; From 1bc14933b76a6fc0b8b82f6a46479ff396be2656 Mon Sep 17 00:00:00 2001 From: Christopher Haster Date: Wed, 17 Apr 2024 00:16:20 -0500 Subject: [PATCH 3/3] Tweaked on-disk config comments for consistency - Prefer "defaults to blablabla when zero" to hint that this is the default state when both explicitly set to zero and implicitly set to zero thanks to C's initializers. - Prefer "disk" when referencing something stored "on disk". Other terms can quickly get ambiguous. Except maybe "block device"... --- lfs.h | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/lfs.h b/lfs.h index 274259e2..84738973 100644 --- a/lfs.h +++ b/lfs.h @@ -59,7 +59,8 @@ typedef uint32_t lfs_block_t; #endif // Maximum size of custom attributes in bytes, may be redefined, but there is -// no real benefit to using a smaller LFS_ATTR_MAX. Limited to <= 1022. +// no real benefit to using a smaller LFS_ATTR_MAX. Limited to <= 1022. Stored +// in superblock and must be respected by other littlefs drivers. #ifndef LFS_ATTR_MAX #define LFS_ATTR_MAX 1022 #endif @@ -203,8 +204,8 @@ struct lfs_config { // program sizes. lfs_size_t block_size; - // Number of erasable blocks on the device. - // If 0, will attempt to infer block_count from existing filesystem. + // Number of erasable blocks on the device. Defaults to block_count stored + // on disk when zero. lfs_size_t block_count; // Number of erase cycles before littlefs evicts metadata logs and moves @@ -253,18 +254,18 @@ struct lfs_config { // Optional upper limit on length of file names in bytes. No downside for // larger names except the size of the info struct which is controlled by - // the LFS_NAME_MAX define. Defaults to LFS_NAME_MAX when zero. Stored in - // superblock and must be respected by other littlefs drivers. + // the LFS_NAME_MAX define. Defaults to LFS_NAME_MAX or name_max stored on + // disk when zero. lfs_size_t name_max; // Optional upper limit on files in bytes. No downside for larger files - // but must be <= LFS_FILE_MAX. Defaults to LFS_FILE_MAX when zero. Stored - // in superblock and must be respected by other littlefs drivers. + // but must be <= LFS_FILE_MAX. Defaults to LFS_FILE_MAX or file_max stored + // on disk when zero. lfs_size_t file_max; // Optional upper limit on custom attributes in bytes. No downside for // larger attributes size but must be <= LFS_ATTR_MAX. Defaults to - // LFS_ATTR_MAX when zero. + // LFS_ATTR_MAX or attr_max stored on disk when zero. lfs_size_t attr_max; // Optional upper limit on total space given to metadata pairs in bytes. On