Github/littlefs
1
Fork 0
Code Actions Activity

Change lfsr_mount to return CORRUPT or NOTSUP

Browse Source 
Before, lfsr_mount would return LFS_ERR_INVAL if it could not mount the
filesystem for any reason. This matches POSIX's mount behavior, but is,
in my humble opinion, unhelpful... A corrupted filesystem image is an
"invalid parameter"?

This splits lfsr_mount's failed-to-mount behavior into two error codes:

- LFS_ERR_CORRUPT - Failed to mount because something was corrupted.
  Unlikely disk contains a littlefs image.

- LFS_ERR_NOTSUP - Failed to mount because on-disk filesystem is
  incompatible. Reconfiguring your driver may successfully mount.

This offers a bit more of a hint to users on why mount failed. Though
relevant error logs will probably have more useful information. Worst
case users can always treat CORRUPT/NOTSUP the same after calling
lfsr_mount.

Code changes:

           code          stack
  before: 33674           2592
  after:  33686 (+0.0%)   2592 (+0.0%)
This commit is contained in:
Christopher Haster
2024-06-09 14:59:05 -05:00
parent c72e3b24da
commit 55f0872dbc
2 changed files with 97 additions and 28 deletions
Download Patch File Download Diff File Expand all files Collapse all files
lfs.c
+13 -13
View File
@@ -5543,7 +5543,7 @@ static int lfsr_data_readgrm(lfs_t *lfs, lfsr_data_t *data,
// unknown mode? return an error, we may be able to mount read-only
if (mode > 2) {
return LFS_ERR_INVAL;
return LFS_ERR_CORRUPT;
}
for (uint8_t i = 0; i < mode; i++) {
@@ -8338,7 +8338,7 @@ static int lfsr_mountmroot(lfs_t *lfs, const lfsr_mdir_t *mroot) {
if (err) {
if (err == LFS_ERR_NOENT) {
LFS_ERROR("No littlefs version found");
return LFS_ERR_INVAL;
return LFS_ERR_CORRUPT;
}
return err;
}
@@ -8355,7 +8355,7 @@ static int lfsr_mountmroot(lfs_t *lfs, const lfsr_mdir_t *mroot) {
version[1],
LFS_DISK_VERSION_MAJOR,
LFS_DISK_VERSION_MINOR);
return LFS_ERR_INVAL;
return LFS_ERR_NOTSUP;
}
// check for any rcompatflags, we must understand these to read
@@ -8378,7 +8378,7 @@ static int lfsr_mountmroot(lfs_t *lfs, const lfsr_mdir_t *mroot) {
" (!= 0x%0"PRIx16")",
rcompat,
LFSR_RCOMPAT_COMPAT);
return LFS_ERR_INVAL;
return LFS_ERR_NOTSUP;
}
// check for any wcompatflags, we must understand these to write
@@ -8402,7 +8402,7 @@ static int lfsr_mountmroot(lfs_t *lfs, const lfsr_mdir_t *mroot) {
" (!= 0x%0"PRIx16")",
wcompat,
LFSR_WCOMPAT_COMPAT);
return LFS_ERR_INVAL;
return LFS_ERR_NOTSUP;
}
// we don't bother to check for any ocompatflags, we would just
@@ -8428,14 +8428,14 @@ static int lfsr_mountmroot(lfs_t *lfs, const lfsr_mdir_t *mroot) {
LFS_ERROR("Incompatible block size %"PRId32" (!= %"PRId32")",
geometry.block_size,
lfs->cfg->block_size);
return LFS_ERR_INVAL;
return LFS_ERR_NOTSUP;
}
if (geometry.block_count != lfs->cfg->block_count) {
LFS_ERROR("Incompatible block count %"PRId32" (!= %"PRId32")",
geometry.block_count,
lfs->cfg->block_count);
return LFS_ERR_INVAL;
return LFS_ERR_NOTSUP;
}
// read the name limit
@@ -8459,7 +8459,7 @@ static int lfsr_mountmroot(lfs_t *lfs, const lfsr_mdir_t *mroot) {
LFS_ERROR("Incompatible name limit (%"PRId32" > %"PRId32")",
name_limit,
lfs->name_limit);
return LFS_ERR_INVAL;
return LFS_ERR_NOTSUP;
}
lfs->name_limit = name_limit;
@@ -8485,7 +8485,7 @@ static int lfsr_mountmroot(lfs_t *lfs, const lfsr_mdir_t *mroot) {
LFS_ERROR("Incompatible file limit (%"PRId32" > %"PRId32")",
file_limit,
lfs->file_limit);
return LFS_ERR_INVAL;
return LFS_ERR_NOTSUP;
}
lfs->file_limit = file_limit;
@@ -8502,7 +8502,7 @@ static int lfsr_mountmroot(lfs_t *lfs, const lfsr_mdir_t *mroot) {
&& lfsr_tag_suptype(tag) == LFSR_TAG_CONFIG) {
LFS_ERROR("Unknown config 0x%04"PRIx16,
tag);
return LFS_ERR_INVAL;
return LFS_ERR_NOTSUP;
}
return 0;
@@ -8543,7 +8543,7 @@ static int lfsr_mountinited(lfs_t *lfs) {
if (err) {
if (err == LFS_ERR_NOENT) {
LFS_ERROR("No littlefs magic found");
return LFS_ERR_INVAL;
return LFS_ERR_CORRUPT;
}
return err;
}
@@ -8555,7 +8555,7 @@ static int lfsr_mountinited(lfs_t *lfs) {
}
if (cmp != LFS_CMP_EQ) {
LFS_ERROR("No littlefs magic found");
return LFS_ERR_INVAL;
return LFS_ERR_CORRUPT;
}
// are we the last mroot?
@@ -8625,7 +8625,7 @@ static int lfsr_mountinited(lfs_t *lfs) {
>> lfs->mdir_bits,
rid,
lfsr_tag_subtype(tag));
return LFS_ERR_INVAL;
return LFS_ERR_NOTSUP;
}
}
tests/test_incompat.toml
+84 -15
View File
@@ -7,6 +7,52 @@ after = ['test_mtree', 'test_dirs', 'test_files']
## incompatiblity tests ##
# test that we fail if we find no magic
[cases.test_incompat_no_magic]
in = 'lfs.c'
code = '''
// create a superblock
lfs_t lfs;
lfsr_format(&lfs, CFG) => 0;
// delete the magic string
//
// note we're messing around with internals to do this! this
// is not a user API
lfsr_mount(&lfs, CFG) => 0;
lfsr_mdir_commit(&lfs, &lfs.mroot, LFSR_ATTRS(
LFSR_ATTR(
LFSR_TAG_RM | LFSR_TAG_MAGIC, 0,
LFSR_DATA_NULL()))) => 0;
lfsr_unmount(&lfs) => 0;
// mount should now fail
lfsr_mount(&lfs, CFG) => LFS_ERR_CORRUPT;
'''
# test that we fail if we find bad magic
[cases.test_incompat_bad_magic]
in = 'lfs.c'
code = '''
// create a superblock
lfs_t lfs;
lfsr_format(&lfs, CFG) => 0;
// tweak the magic string
//
// note we're messing around with internals to do this! this
// is not a user API
lfsr_mount(&lfs, CFG) => 0;
lfsr_mdir_commit(&lfs, &lfs.mroot, LFSR_ATTRS(
LFSR_ATTR(
LFSR_TAG_MAGIC, 0,
LFSR_DATA_BUF("lottlefs", 8)))) => 0;
lfsr_unmount(&lfs) => 0;
// mount should now fail
lfsr_mount(&lfs, CFG) => LFS_ERR_CORRUPT;
'''
# test that we fail to mount after a major version bump
[cases.test_incompat_major]
in = 'lfs.c'
@@ -29,7 +75,7 @@ code = '''
lfsr_unmount(&lfs) => 0;
// mount should now fail
lfsr_mount(&lfs, CFG) => LFS_ERR_INVAL;
lfsr_mount(&lfs, CFG) => LFS_ERR_NOTSUP;
'''
# test that we fail to mount after a minor version bump
@@ -54,7 +100,7 @@ code = '''
lfsr_unmount(&lfs) => 0;
// mount should now fail
lfsr_mount(&lfs, CFG) => LFS_ERR_INVAL;
lfsr_mount(&lfs, CFG) => LFS_ERR_NOTSUP;
'''
# test that we fail to mount incompatible rcompat flags
@@ -80,7 +126,7 @@ code = '''
lfsr_unmount(&lfs) => 0;
// mount should now fail
lfsr_mount(&lfs, CFG) => LFS_ERR_INVAL;
lfsr_mount(&lfs, CFG) => LFS_ERR_NOTSUP;
'''
# test that we fail to mount incompatible wcompat flags
@@ -106,7 +152,7 @@ code = '''
lfsr_unmount(&lfs) => 0;
// mount should now fail
lfsr_mount(&lfs, CFG) => LFS_ERR_INVAL;
lfsr_mount(&lfs, CFG) => LFS_ERR_NOTSUP;
'''
# test that an incompatible ocompat flag is a noop
@@ -157,7 +203,7 @@ code = '''
lfsr_unmount(&lfs) => 0;
// mount should now fail
lfsr_mount(&lfs, CFG) => LFS_ERR_INVAL;
lfsr_mount(&lfs, CFG) => LFS_ERR_NOTSUP;
'''
[cases.test_incompat_wcompat_overflow]
@@ -180,7 +226,7 @@ code = '''
lfsr_unmount(&lfs) => 0;
// mount should now fail
lfsr_mount(&lfs, CFG) => LFS_ERR_INVAL;
lfsr_mount(&lfs, CFG) => LFS_ERR_NOTSUP;
'''
[cases.test_incompat_ocompat_overflow]
@@ -230,7 +276,7 @@ code = '''
lfsr_unmount(&lfs) => 0;
// mount should now fail
lfsr_mount(&lfs, CFG) => LFS_ERR_INVAL;
lfsr_mount(&lfs, CFG) => LFS_ERR_NOTSUP;
'''
# test that we fail to mount after incompatible block counts
@@ -256,7 +302,7 @@ code = '''
lfsr_unmount(&lfs) => 0;
// mount should now fail
lfsr_mount(&lfs, CFG) => LFS_ERR_INVAL;
lfsr_mount(&lfs, CFG) => LFS_ERR_NOTSUP;
'''
# test that we fail to mount after incompatible name limit
@@ -280,7 +326,7 @@ code = '''
lfsr_unmount(&lfs) => 0;
// mount should now fail
lfsr_mount(&lfs, CFG) => LFS_ERR_INVAL;
lfsr_mount(&lfs, CFG) => LFS_ERR_NOTSUP;
'''
# test that we fail to mount after incompatible file limit
@@ -308,11 +354,31 @@ code = '''
lfsr_unmount(&lfs) => 0;
// mount should now fail
lfsr_mount(&lfs, CFG) => LFS_ERR_INVAL;
lfsr_mount(&lfs, CFG) => LFS_ERR_NOTSUP;
'''
# test what happens if we find an unknown config
[cases.test_incompat_unknown_config]
in = 'lfs.c'
code = '''
// create a superblock
lfs_t lfs;
lfsr_format(&lfs, CFG) => 0;
// create an unknown config
lfsr_mount(&lfs, CFG) => 0;
lfsr_mdir_commit(&lfs, &lfs.mroot, LFSR_ATTRS(
LFSR_ATTR(
LFSR_TAG_CONFIG + 0x13, 0,
LFSR_DATA_BUF("oh no!", strlen("oh no!"))))) => 0;
lfsr_unmount(&lfs) => 0;
// mount should now fail
lfsr_mount(&lfs, CFG) => LFS_ERR_NOTSUP;
'''
# test what happens if we find an unknown file type
[cases.test_incompat_unknown]
[cases.test_incompat_unknown_type]
in = 'lfs.c'
code = '''
// create a superblock
@@ -324,15 +390,18 @@ code = '''
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_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, "hi b!", strlen("hi b!")) => strlen("hi b!");
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_write(&lfs, &file,
"hi c!", strlen("hi c!")) => strlen("hi c!");
lfsr_file_close(&lfs, &file) => 0;
lfsr_unmount(&lfs) => 0;
@@ -352,5 +421,5 @@ code = '''
lfsr_unmount(&lfs) => 0;
// mount should now fail
lfsr_mount(&lfs, CFG) => LFS_ERR_INVAL;
lfsr_mount(&lfs, CFG) => LFS_ERR_NOTSUP;
'''