Github/littlefs
1
Fork 0
Code Actions Activity

Don't propagate grm cleanup errors

Browse Source 
While it may be useful to know when/why lfsr_fs_fixgrm fails, at this
point in lfsr_rename/lfsr_remove the operation has already succeeded as
far as the filesystem is concerned.

It's counterintuitive, but ignoring these errors actually tells the user
_more_ information, specifically whether or not the operation completed
on disk.

At least we can log the error via LFS_WARN, and such errors will likely
come up again in a future operation, such as the call to lfsr_fs_fixgrm
on the next filesystem mutation.

This was noticed in test_grow, which tests error code-paths quite a bit
more than any other test.

Code changes:

           code          stack
  before: 33934           2592
  after:  33942 (+0.0%)   2592 (+0.0%)
This commit is contained in:
Christopher Haster
2024-06-12 01:40:07 -05:00
parent 9f5cc0582a
commit d64cfc7eaa
2 changed files with 30 additions and 18 deletions
Download Patch File Download Diff File Expand all files Collapse all files
lfs.c
+16 -2
View File
@@ -9536,7 +9536,14 @@ int lfsr_remove(lfs_t *lfs, const char *path) {
// if we were a directory, we need to clean up, fortunately we can leave // if we were a directory, we need to clean up, fortunately we can leave
// this up to lfsr_fs_fixgrm // this up to lfsr_fs_fixgrm
return lfsr_fs_fixgrm(lfs); err = lfsr_fs_fixgrm(lfs);
if (err) {
// we did complete the remove, so we shouldn't error here, best
// we can do is log this
LFS_WARN("Failed to clean up grm (%d)", err);
}
return 0;
} }
int lfsr_rename(lfs_t *lfs, const char *old_path, const char *new_path) { int lfsr_rename(lfs_t *lfs, const char *old_path, const char *new_path) {
@@ -9705,7 +9712,14 @@ int lfsr_rename(lfs_t *lfs, const char *old_path, const char *new_path) {
// we need to clean up any pending grms, fortunately we can leave // we need to clean up any pending grms, fortunately we can leave
// this up to lfsr_fs_fixgrm // this up to lfsr_fs_fixgrm
return lfsr_fs_fixgrm(lfs); err = lfsr_fs_fixgrm(lfs);
if (err) {
// we did complete the remove, so we shouldn't error here, best
// we can do is log this
LFS_WARN("Failed to clean up grm (%d)", err);
}
return 0;
} }
// this just populates the info struct based on what we found // this just populates the info struct based on what we found
tests/test_grow.toml
+14 -16
View File
@@ -658,7 +658,7 @@ code = '''
char name[256]; char name[256];
sprintf(name, "dir%03x", x); sprintf(name, "dir%03x", x);
int err = lfsr_remove(&lfs, name); int err = lfsr_remove(&lfs, name);
assert(!err || err == LFS_ERR_NOSPC || err == LFS_ERR_NOENT); assert(!err || err == LFS_ERR_NOSPC);
if (err == LFS_ERR_NOSPC) { if (err == LFS_ERR_NOSPC) {
goto grow; goto grow;
} }
@@ -681,7 +681,7 @@ code = '''
char new_name[256]; char new_name[256];
sprintf(new_name, "dir%03x", y); sprintf(new_name, "dir%03x", y);
int err = lfsr_rename(&lfs, old_name, new_name); int err = lfsr_rename(&lfs, old_name, new_name);
assert(!err || err == LFS_ERR_NOSPC || err == LFS_ERR_NOENT); assert(!err || err == LFS_ERR_NOSPC);
if (err == LFS_ERR_NOSPC) { if (err == LFS_ERR_NOSPC) {
goto grow; goto grow;
} }
@@ -1070,7 +1070,7 @@ code = '''
char name[256]; char name[256];
sprintf(name, "amethyst%03x", x); sprintf(name, "amethyst%03x", x);
int err = lfsr_remove(&lfs, name); int err = lfsr_remove(&lfs, name);
assert(!err || err == LFS_ERR_NOSPC || err == LFS_ERR_NOENT); assert(!err || err == LFS_ERR_NOSPC);
if (err == LFS_ERR_NOSPC) { if (err == LFS_ERR_NOSPC) {
goto grow; goto grow;
} }
@@ -1097,7 +1097,7 @@ code = '''
char new_name[256]; char new_name[256];
sprintf(new_name, "amethyst%03x", y); sprintf(new_name, "amethyst%03x", y);
int err = lfsr_rename(&lfs, old_name, new_name); int err = lfsr_rename(&lfs, old_name, new_name);
assert(!err || err == LFS_ERR_NOSPC || err == LFS_ERR_NOENT); assert(!err || err == LFS_ERR_NOSPC);
if (err == LFS_ERR_NOSPC) { if (err == LFS_ERR_NOSPC) {
goto grow; goto grow;
} }
@@ -1469,7 +1469,7 @@ code = '''
char name[256]; char name[256];
sprintf(name, "batman%03x", x); sprintf(name, "batman%03x", x);
int err = lfsr_remove(&lfs, name); int err = lfsr_remove(&lfs, name);
assert(!err || err == LFS_ERR_NOSPC || err == LFS_ERR_NOENT); assert(!err || err == LFS_ERR_NOSPC);
if (err == LFS_ERR_NOSPC) { if (err == LFS_ERR_NOSPC) {
goto grow; goto grow;
} }
@@ -1506,7 +1506,7 @@ code = '''
char new_name[256]; char new_name[256];
sprintf(new_name, "batman%03x", y); sprintf(new_name, "batman%03x", y);
int err = lfsr_rename(&lfs, old_name, new_name); int err = lfsr_rename(&lfs, old_name, new_name);
assert(!err || err == LFS_ERR_NOSPC || err == LFS_ERR_NOENT); assert(!err || err == LFS_ERR_NOSPC);
if (err == LFS_ERR_NOSPC) { if (err == LFS_ERR_NOSPC) {
goto grow; goto grow;
} }
@@ -1902,7 +1902,7 @@ code = '''
char name[256]; char name[256];
sprintf(name, "batman%03x", x); sprintf(name, "batman%03x", x);
int err = lfsr_remove(&lfs, name); int err = lfsr_remove(&lfs, name);
assert(!err || err == LFS_ERR_NOSPC || err == LFS_ERR_NOENT); assert(!err || err == LFS_ERR_NOSPC);
if (err == LFS_ERR_NOSPC) { if (err == LFS_ERR_NOSPC) {
goto grow; goto grow;
} }
@@ -1955,7 +1955,7 @@ code = '''
char new_name[256]; char new_name[256];
sprintf(new_name, "batman%03x", y); sprintf(new_name, "batman%03x", y);
int err = lfsr_rename(&lfs, old_name, new_name); int err = lfsr_rename(&lfs, old_name, new_name);
assert(!err || err == LFS_ERR_NOSPC || err == LFS_ERR_NOENT); assert(!err || err == LFS_ERR_NOSPC);
if (err == LFS_ERR_NOSPC) { if (err == LFS_ERR_NOSPC) {
goto grow; goto grow;
} }
@@ -2363,7 +2363,7 @@ code = '''
assert(strlen(info.name) == strlen("amethyst...")); assert(strlen(info.name) == strlen("amethyst..."));
sprintf(name, "test/%s", info.name); sprintf(name, "test/%s", info.name);
err = lfsr_remove(&lfs, name); err = lfsr_remove(&lfs, name);
assert(!err || err == LFS_ERR_NOSPC || err == LFS_ERR_NOENT); assert(!err || err == LFS_ERR_NOSPC);
if (err == LFS_ERR_NOSPC) { if (err == LFS_ERR_NOSPC) {
goto grow; goto grow;
} }
@@ -2390,7 +2390,7 @@ code = '''
char new_name[256]; char new_name[256];
sprintf(new_name, "test/amethyst%03x", y); sprintf(new_name, "test/amethyst%03x", y);
err = lfsr_rename(&lfs, old_name, new_name); err = lfsr_rename(&lfs, old_name, new_name);
assert(!err || err == LFS_ERR_NOSPC || err == LFS_ERR_NOENT); assert(!err || err == LFS_ERR_NOSPC);
if (err == LFS_ERR_NOSPC) { if (err == LFS_ERR_NOSPC) {
goto grow; goto grow;
} }
@@ -2657,8 +2657,7 @@ code = '''
err = lfsr_remove(&lfs, name); err = lfsr_remove(&lfs, name);
assert(!err assert(!err
|| err == LFS_ERR_NOTEMPTY || err == LFS_ERR_NOTEMPTY
|| err == LFS_ERR_NOSPC || err == LFS_ERR_NOSPC);
|| err == LFS_ERR_NOENT);
if (err == LFS_ERR_NOSPC) { if (err == LFS_ERR_NOSPC) {
goto grow; goto grow;
} }
@@ -2687,8 +2686,7 @@ code = '''
err = lfsr_rename(&lfs, old_name, new_name); err = lfsr_rename(&lfs, old_name, new_name);
assert(!err assert(!err
|| err == LFS_ERR_NOTEMPTY || err == LFS_ERR_NOTEMPTY
|| err == LFS_ERR_NOSPC || err == LFS_ERR_NOSPC);
|| err == LFS_ERR_NOENT);
if (err == LFS_ERR_NOSPC) { if (err == LFS_ERR_NOSPC) {
goto grow; goto grow;
} }
@@ -2793,7 +2791,7 @@ code = '''
assert(strlen(info.name) == strlen("amethyst...")); assert(strlen(info.name) == strlen("amethyst..."));
sprintf(name, "%s/%s", dir_path, info.name); sprintf(name, "%s/%s", dir_path, info.name);
err = lfsr_remove(&lfs, name); err = lfsr_remove(&lfs, name);
assert(!err || err == LFS_ERR_NOSPC || err == LFS_ERR_NOENT); assert(!err || err == LFS_ERR_NOSPC);
if (err == LFS_ERR_NOSPC) { if (err == LFS_ERR_NOSPC) {
goto grow; goto grow;
} }
@@ -2833,7 +2831,7 @@ code = '''
char new_name[256]; char new_name[256];
sprintf(new_name, "test/%s/amethyst%03x", info_.name, y); sprintf(new_name, "test/%s/amethyst%03x", info_.name, y);
err = lfsr_rename(&lfs, old_name, new_name); err = lfsr_rename(&lfs, old_name, new_name);
assert(!err || err == LFS_ERR_NOSPC || err == LFS_ERR_NOENT); assert(!err || err == LFS_ERR_NOSPC);
if (err == LFS_ERR_NOSPC) { if (err == LFS_ERR_NOSPC) {
goto grow; goto grow;
} }