Github/littlefs
1
Fork 0
Code Actions Activity

Added better error handling of unknown filetypes

Browse Source 
This is the tradeoff of not erroring on unknown filetypes during mount.

- lfsr_file_open and lfsr_mtree_pathlookup now returns LFS_ERR_NOTSUP
  instead of LFS_ERR_NOTDIR/LFS_ERR_ISDIR if it encounters an unkown
  filetype.

  This gets a bit subtle. You might think LFS_ERR_NOTDIR is reasonable,
  but it's possible for our unknown filetype to be something dir-like.

  Symlinks are an excellent example.

- lfsr_remove/lfsr_rename now bail with LFS_ERR_NOTSUP if encountering
  an unknown filetype.

  This conflicts with the POSIX philosophy of remove always being
  allowed, but I'm not sure what other option there is. Maybe allowing
  removes when mounted with LFS_M_FORCE?

  We can't just allow removes by default because of the risk of leaking
  resources. Directories being the main example of this (need to clean
  up bookmarks).

  Maybe leaky filetypes should also set WCOMPAT flags?

Not doing something is cheaper than doing something, so unfortunately
this costs us more than what we saved from dropping the orphan/unknown
scan during mount:

                              code          stack          ctx
  bail:                      38120           2624          725
  no-error-no-bail (before): 38020 (-0.3%)   2624 (+0.0%)  752 (+0.0%)
  error-no-bail (after):     38140 (+0.1%)   2624 (+0.0%)  752 (+0.0%)

But this is probably worth it for the extra flexibility.
This commit is contained in:
Christopher Haster
2024-12-30 22:38:35 -06:00
parent 6e63920338
commit 1a99c195f0
2 changed files with 113 additions and 3 deletions
Download Patch File Download Diff File Expand all files Collapse all files
lfs.c
+20 -3
View File
@@ -9523,6 +9523,7 @@ static inline bool lfsr_path_isdir(const char *path) {
// - LFS_ERR_NOENT, lfsr_path_islast(path) => file not found
// - LFS_ERR_NOENT, !lfsr_path_islast(path) => parent not found
// - LFS_ERR_NOTDIR => parent not a dir
// - LFS_ERR_NOTSUP => parent of unknown type
//
// if not found, mdir_/did_ will at least be set up with what should be
// the parent
@@ -9603,8 +9604,10 @@ static int lfsr_mtree_pathlookup(lfs_t *lfs, const char **path,
// only continue if we hit a directory
if (tag != LFSR_TAG_DIR) {
return (tag == LFSR_TAG_STICKYNOTE)
? LFS_ERR_NOENT
: LFS_ERR_NOTDIR;
? LFS_ERR_NOENT
: (lfsr_tag_isunknown(tag))
? LFS_ERR_NOTSUP
: LFS_ERR_NOTDIR;
}
// read the next did from the mdir if this is not the root
@@ -10606,6 +10609,10 @@ int lfsr_remove(lfs_t *lfs, const char *path) {
if (tag == LFSR_TAG_STICKYNOTE) {
return LFS_ERR_NOENT;
}
// we can't remove unknown types or else we may leak resources
if (lfsr_tag_isunknown(tag)) {
return LFS_ERR_NOTSUP;
}
// trying to remove the root dir?
if (mdir.mid == -1) {
@@ -10725,6 +10732,10 @@ int lfsr_rename(lfs_t *lfs, const char *old_path, const char *new_path) {
if (old_tag == LFSR_TAG_STICKYNOTE) {
return LFS_ERR_NOENT;
}
// we can't rename unknown types or else we may leak resources
if (lfsr_tag_isunknown(old_tag)) {
return LFS_ERR_NOTSUP;
}
// trying to rename the root?
if (old_mdir.mid == -1) {
@@ -10740,6 +10751,10 @@ int lfsr_rename(lfs_t *lfs, const char *old_path, const char *new_path) {
if (err && !(err == LFS_ERR_NOENT && lfsr_path_islast(new_path))) {
return err;
}
// we can't rename unknown types or else we may leak resources
if (lfsr_tag_isunknown(new_tag)) {
return LFS_ERR_NOTSUP;
}
// already exists?
bool exists = (err != LFS_ERR_NOENT);
@@ -11616,7 +11631,9 @@ int lfsr_file_opencfg(lfs_t *lfs, lfsr_file_t *file,
// wrong type?
if (tag != LFSR_TAG_REG) {
return LFS_ERR_ISDIR;
return (tag == LFSR_TAG_DIR)
? LFS_ERR_ISDIR
: LFS_ERR_NOTSUP;
}
}
tests/test_mount.toml
+93
View File
@@ -902,3 +902,96 @@ code = '''
lfsr_mount(&lfs, LFS_M_RDONLY, CFG) => LFS_ERR_NOTSUP;
'''
# test what happens if we find an unknown file type
[cases.test_mount_incompat_unknown_type]
in = 'lfs.c'
code = '''
// create a superblock
lfs_t lfs;
lfsr_format(&lfs, LFS_F_RDWR, CFG) => 0;
// create some files
lfsr_mount(&lfs, LFS_M_RDWR, CFG) => 0;
lfsr_file_t file;
lfsr_file_open(&lfs, &file, "a",
LFS_O_WRONLY | LFS_O_CREAT | LFS_O_EXCL) => 0;
lfsr_file_write(&lfs, &file,
"hi a!", strlen("hi a!")) => strlen("hi a!");
lfsr_file_close(&lfs, &file) => 0;
lfsr_file_open(&lfs, &file, "b",
LFS_O_WRONLY | LFS_O_CREAT | LFS_O_EXCL) => 0;
lfsr_file_write(&lfs, &file,
"oh no!", strlen("oh no!")) => strlen("oh no!");
lfsr_file_close(&lfs, &file) => 0;
lfsr_file_open(&lfs, &file, "c",
LFS_O_WRONLY | LFS_O_CREAT | LFS_O_EXCL) => 0;
lfsr_file_write(&lfs, &file,
"hi c!", strlen("hi c!")) => strlen("hi c!");
lfsr_file_close(&lfs, &file) => 0;
lfsr_unmount(&lfs) => 0;
// change a file's type to something unknown
lfsr_mount(&lfs, LFS_M_RDWR, CFG) => 0;
const char *path = "b";
lfsr_mdir_t mdir;
lfsr_did_t did;
lfsr_mtree_pathlookup(&lfs, &path,
&mdir, NULL, &did) => 0;
lfsr_mdir_commit(&lfs, &mdir, LFSR_RATS(
LFSR_RAT_NAME(
LFSR_TAG_SUB | (LFSR_TAG_NAME + 0x13), 0,
did, path, lfsr_path_namelen(path)))) => 0;
lfsr_unmount(&lfs) => 0;
// mount
lfsr_mount(&lfs, LFS_M_RDWR, 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;
lfsr_unmount(&lfs) => 0;
'''