From c72e3b24da3639b28af780c2e754e069625a4f59 Mon Sep 17 00:00:00 2001 From: Christopher Haster Date: Sun, 9 Jun 2024 14:18:02 -0500 Subject: [PATCH] Made unknown file types a hard mount error, reverting I realized we really can't do anything if we find a file of unknown type... If we don't understand a file's data structure, we can't really do any bookkeeping. Allocating new blocks will probably corrupt unknown files since we can't traverse any related B-trees, and mdir compaction would be an absolute mess. So, instead, just print an error and bail during mount. Eventually we could at least fallback to readonly mode, but this is currently a TODO item. This also means the LFS_ERR_NOTSUP logic in lfsr_mtree_pathlookup is no longer needed. Since, even with readonly fallback, we should never mutate a filesystem with unknown file types. Maybe in the future we could have a sort of known-but-not-supported mode for file types? So special file types could not be support, but at least understood enough to support traversal/remove/rename/etc? Code changes: code stack before: 33694 2592 after: 33674 (-0.1%) 2592 (+0.0%) --- lfs.c | 18 +++++------ tests/test_incompat.toml | 67 ++-------------------------------------- 2 files changed, 9 insertions(+), 76 deletions(-) diff --git a/lfs.c b/lfs.c index b8dbc755..cbc95bc8 100644 --- a/lfs.c +++ b/lfs.c @@ -7493,7 +7493,6 @@ enum { // - 0 => path is valid, file NOT found // - EXIST => path is valid, file found // - INVAL => path is valid, but points to root -// - NOTSUP => path is valid, but points to unknown file type // - NOENT => path is NOT valid, intermediate dir missing // - NOTDIR => path is NOT valid, intermediate dir is not a dir // @@ -7567,11 +7566,8 @@ static int lfsr_mtree_pathlookup(lfs_t *lfs, const lfsr_mtree_t *mtree, // the root dir doesn't have an mdir really, so it's always // a special case return (mdir.mid == -1) - ? LFS_ERR_INVAL - // unknown file type? - : (lfsr_tag_isunknown(tag)) - ? LFS_ERR_NOTSUP - : LFS_ERR_EXIST; + ? LFS_ERR_INVAL + : LFS_ERR_EXIST; } // found another name @@ -8622,12 +8618,14 @@ static int lfsr_mountinited(lfs_t *lfs) { // found an unknown file type? } else if (lfsr_tag_isunknown(tag)) { - LFS_WARN("Found unknown file type " + // TODO switch to readonly? + LFS_ERROR("Found unknown file type " "%"PRId32".%"PRId32" 0x%"PRIx16, lfsr_mid_bid(lfs, tinfo.u.mdir.mid) >> lfs->mdir_bits, rid, lfsr_tag_subtype(tag)); + return LFS_ERR_INVAL; } } @@ -9161,8 +9159,7 @@ int lfsr_mkdir(lfs_t *lfs, const char *path) { &mdir, &tag, &did, &name, &name_size); if (err && err != LFS_ERR_EXIST - && err != LFS_ERR_INVAL - && err != LFS_ERR_NOTSUP) { + && err != LFS_ERR_INVAL) { return err; } // already exists? note orphans don't really exist @@ -9660,8 +9657,7 @@ int lfsr_stat(lfs_t *lfs, const char *path, struct lfs_info *info) { &mdir, &tag, NULL, &name, &name_size); if (err && err != LFS_ERR_EXIST - && err != LFS_ERR_INVAL - && err != LFS_ERR_NOTSUP) { + && err != LFS_ERR_INVAL) { return err; } // doesn't exist? note orphans don't really exist diff --git a/tests/test_incompat.toml b/tests/test_incompat.toml index be44b59a..313cfc80 100644 --- a/tests/test_incompat.toml +++ b/tests/test_incompat.toml @@ -351,69 +351,6 @@ code = ''' did, name, name_size))) => 0; lfsr_unmount(&lfs) => 0; - // mount - lfsr_mount(&lfs, CFG) => 0; - - // our file should appear as an unknown type - struct lfs_info info; - lfsr_stat(&lfs, "b", &info) => 0; - assert(strcmp(info.name, "b") == 0); - assert(info.type == 0x13); - assert(info.size == 0); - - lfsr_dir_t dir; - lfsr_dir_open(&lfs, &dir, "/") => 0; - lfsr_dir_read(&lfs, &dir, &info) => 0; - assert(strcmp(info.name, ".") == 0); - assert(info.type == LFS_TYPE_DIR); - assert(info.size == 0); - lfsr_dir_read(&lfs, &dir, &info) => 0; - assert(strcmp(info.name, "..") == 0); - assert(info.type == LFS_TYPE_DIR); - assert(info.size == 0); - lfsr_dir_read(&lfs, &dir, &info) => 0; - assert(strcmp(info.name, "a") == 0); - assert(info.type == LFS_TYPE_REG); - assert(info.size == strlen("hi a!")); - lfsr_dir_read(&lfs, &dir, &info) => 0; - assert(strcmp(info.name, "b") == 0); - assert(info.type == 0x13); - assert(info.size == 0); - lfsr_dir_read(&lfs, &dir, &info) => 0; - assert(strcmp(info.name, "c") == 0); - assert(info.type == LFS_TYPE_REG); - assert(info.size == strlen("hi c!")); - lfsr_dir_read(&lfs, &dir, &info) => LFS_ERR_NOENT; - lfsr_dir_close(&lfs, &dir) => 0; - - // removing/renaming unknown files should return NOTSUP, we could - // remove the metadata entry, but we would probably leak stuff - lfsr_remove(&lfs, "b") => LFS_ERR_NOTSUP; - lfsr_rename(&lfs, "b", "d") => LFS_ERR_NOTSUP; - lfsr_rename(&lfs, "b", "c") => LFS_ERR_NOTSUP; - lfsr_rename(&lfs, "a", "b") => LFS_ERR_NOTSUP; - - lfsr_file_open(&lfs, &file, "b", - LFS_O_RDONLY) => LFS_ERR_NOTSUP; - lfsr_file_open(&lfs, &file, "b", - LFS_O_WRONLY | LFS_O_CREAT) => LFS_ERR_NOTSUP; - - lfsr_mkdir(&lfs, "b") => LFS_ERR_EXIST; - - // we should also not try to use unknown files as dirs, which can - // be a bit tricky - lfsr_stat(&lfs, "b/d", &info) => LFS_ERR_NOTDIR; - lfsr_remove(&lfs, "b/e") => LFS_ERR_NOTDIR; - lfsr_rename(&lfs, "b/e", "d") => LFS_ERR_NOTDIR; - lfsr_rename(&lfs, "b/e", "c") => LFS_ERR_NOTDIR; - lfsr_rename(&lfs, "a", "b/e") => LFS_ERR_NOTDIR; - - lfsr_file_open(&lfs, &file, "b/e", - LFS_O_RDONLY) => LFS_ERR_NOTDIR; - lfsr_file_open(&lfs, &file, "b/e", - LFS_O_WRONLY | LFS_O_CREAT) => LFS_ERR_NOTDIR; - - lfsr_mkdir(&lfs, "b/e") => LFS_ERR_NOTDIR; - - lfsr_unmount(&lfs) => 0; + // mount should now fail + lfsr_mount(&lfs, CFG) => LFS_ERR_INVAL; '''