From b5e503ca856d6779a6f24187c05d996f56e43372 Mon Sep 17 00:00:00 2001 From: Christopher Haster Date: Fri, 25 Apr 2025 17:29:34 -0500 Subject: [PATCH] Made lfsr_file_sync a noop if zombied So now calling lfsr_file_sync on zombied files is a noop: // create a file lfsr_file_t a; lfsr_file_open(&lfs, &a, "a", LFS_O_RDWR | LFS_O_CREAT | LFS_O_EXCL) => 0; // remove, creating a zombie lfsr_remove(&lfs, "a") => 0; // sync, this is now a noop (previously LFS_ERR_NOENT) lfsr_file_sync(&lfs, &a) => 0; // close is also a noop lfsr_file_close(&lfs, &a) => 0; I've been on the fence on this for a while, on one hand erroring provides more information to the user, on the other hand a noop is less surprising if the user comes from other systems. Ended up making this a noop. I figured minimizing surprises is good API design, and the user can always use lfsr_stat to check if the file still exists. This also matches POSIX, and, perhaps more importantly, the current version of littlefs. --- Note that lfsr_file_resync still errors with LFS_ERR_NOENT. It's hard to argue the file "matches the state of disk" otherwise. Code changes minimal: code stack ctx before: 35784 2440 640 after: 35780 (-0.0%) 2440 (+0.0%) 640 (+0.0%) --- lfs.c | 5 ++-- tests/test_attrs.toml | 10 +++---- tests/test_badblocks.toml | 18 ++++-------- tests/test_ck.toml | 6 ++-- tests/test_exhaustion.toml | 6 ++-- tests/test_gc.toml | 6 ++-- tests/test_grow.toml | 6 ++-- tests/test_relocations.toml | 6 ++-- tests/test_stickynotes.toml | 56 ++++++++++++++++++++++--------------- tests/test_traversal.toml | 6 ++-- 10 files changed, 59 insertions(+), 66 deletions(-) diff --git a/lfs.c b/lfs.c index 71f762c6..0550fc49 100644 --- a/lfs.c +++ b/lfs.c @@ -12721,11 +12721,10 @@ int lfsr_file_sync(lfs_t *lfs, lfsr_file_t *file) { // lfsr_file_resync LFS_ASSERT(!lfsr_o_isrdonly(file->b.o.flags)); - // removed? we can't sync + // removed? sync is just a noop in this case int err; if (lfsr_o_iszombie(file->b.o.flags)) { - err = LFS_ERR_NOENT; - goto failed; + return 0; } // first flush any data in our cache, this is a noop if already diff --git a/tests/test_attrs.toml b/tests/test_attrs.toml index c0f85ca4..c4623358 100644 --- a/tests/test_attrs.toml +++ b/tests/test_attrs.toml @@ -3979,10 +3979,10 @@ code = ''' b_size[1] = strlen(b_); c_size[1] = strlen(c_); - // attempting to sync should error - lfsr_file_sync(&lfs, &file[1]) => LFS_ERR_NOENT; + // attempting to sync the zombie file is a noop + lfsr_file_sync(&lfs, &file[1]) => 0; - // attempting to resync should error + // attempting to resync the zombie file should error lfsr_file_resync(&lfs, &file[1]) => LFS_ERR_NOENT; // other file unaffected? @@ -4239,8 +4239,8 @@ code = ''' } } - // attempting to sync the zombie file should error - lfsr_file_sync(&lfs, &file[0]) => LFS_ERR_NOENT; + // attempting to sync the zombie file is a noop + lfsr_file_sync(&lfs, &file[0]) => 0; // attempting to resync the zombie file should error lfsr_file_resync(&lfs, &file[0]) => LFS_ERR_NOENT; diff --git a/tests/test_badblocks.toml b/tests/test_badblocks.toml index 52bfb04f..b600d231 100644 --- a/tests/test_badblocks.toml +++ b/tests/test_badblocks.toml @@ -1131,8 +1131,7 @@ code = ''' wbuf[k] = 'a' + (TEST_PRNG(&wprng_) % 26); } lfsr_file_write(&lfs, &sim_files[j]->file, wbuf, SIZE) => SIZE; - lfsr_file_sync(&lfs, &sim_files[j]->file) - => (!sim_files[j]->zombie) ? 0 : LFS_ERR_NOENT; + lfsr_file_sync(&lfs, &sim_files[j]->file) => 0; // update sim sim_files[j]->prng = wprng; @@ -1591,8 +1590,7 @@ code = ''' wbuf[k] = 'a' + (TEST_PRNG(&wprng_) % 26); } lfsr_file_write(&lfs, &sim_files[j]->file, wbuf, SIZE) => SIZE; - lfsr_file_sync(&lfs, &sim_files[j]->file) - => (!sim_files[j]->zombie) ? 0 : LFS_ERR_NOENT; + lfsr_file_sync(&lfs, &sim_files[j]->file) => 0; // update sim sim_files[j]->prng = wprng; @@ -3094,8 +3092,7 @@ code = ''' wbuf[k] = 'a' + (TEST_PRNG(&wprng_) % 26); } lfsr_file_write(&lfs, &sim_files[j]->file, wbuf, SIZE) => SIZE; - lfsr_file_sync(&lfs, &sim_files[j]->file) - => (!sim_files[j]->zombie) ? 0 : LFS_ERR_NOENT; + lfsr_file_sync(&lfs, &sim_files[j]->file) => 0; // update sim sim_files[j]->prng = wprng; @@ -3555,8 +3552,7 @@ code = ''' wbuf[k] = 'a' + (TEST_PRNG(&wprng_) % 26); } lfsr_file_write(&lfs, &sim_files[j]->file, wbuf, SIZE) => SIZE; - lfsr_file_sync(&lfs, &sim_files[j]->file) - => (!sim_files[j]->zombie) ? 0 : LFS_ERR_NOENT; + lfsr_file_sync(&lfs, &sim_files[j]->file) => 0; // update sim sim_files[j]->prng = wprng; @@ -5051,8 +5047,7 @@ code = ''' wbuf[k] = 'a' + (TEST_PRNG(&wprng_) % 26); } lfsr_file_write(&lfs, &sim_files[j]->file, wbuf, SIZE) => SIZE; - lfsr_file_sync(&lfs, &sim_files[j]->file) - => (!sim_files[j]->zombie) ? 0 : LFS_ERR_NOENT; + lfsr_file_sync(&lfs, &sim_files[j]->file) => 0; // update sim sim_files[j]->prng = wprng; @@ -5512,8 +5507,7 @@ code = ''' wbuf[k] = 'a' + (TEST_PRNG(&wprng_) % 26); } lfsr_file_write(&lfs, &sim_files[j]->file, wbuf, SIZE) => SIZE; - lfsr_file_sync(&lfs, &sim_files[j]->file) - => (!sim_files[j]->zombie) ? 0 : LFS_ERR_NOENT; + lfsr_file_sync(&lfs, &sim_files[j]->file) => 0; // update sim sim_files[j]->prng = wprng; diff --git a/tests/test_ck.toml b/tests/test_ck.toml index 271c600f..be60433f 100644 --- a/tests/test_ck.toml +++ b/tests/test_ck.toml @@ -3746,8 +3746,7 @@ code = ''' goto corrupt_mounted; } int err = lfsr_file_sync(&lfs, &sim_files[j]->file); - assert(err == ((!sim_files[j]->zombie) ? 0 : LFS_ERR_NOENT) - || err == LFS_ERR_NOSPC); + assert(err == 0 || err == LFS_ERR_NOSPC); if (err == LFS_ERR_NOSPC) { goto corrupt_mounted; } @@ -4345,8 +4344,7 @@ code = ''' goto corrupt_mounted; } int err = lfsr_file_sync(&lfs, &sim_files[j]->file); - assert(err == ((!sim_files[j]->zombie) ? 0 : LFS_ERR_NOENT) - || err == LFS_ERR_NOSPC); + assert(err == 0 || err == LFS_ERR_NOSPC); if (err == LFS_ERR_NOSPC) { goto corrupt_mounted; } diff --git a/tests/test_exhaustion.toml b/tests/test_exhaustion.toml index 6d78db93..f0354d09 100644 --- a/tests/test_exhaustion.toml +++ b/tests/test_exhaustion.toml @@ -886,8 +886,7 @@ code = ''' goto dead; } int err = lfsr_file_sync(&lfs, &sim_files[j]->file); - assert(err == ((!sim_files[j]->zombie) ? 0 : LFS_ERR_NOENT) - || err == LFS_ERR_NOSPC); + assert(err == 0 || err == LFS_ERR_NOSPC); if (err == LFS_ERR_NOSPC) { goto dead; } @@ -1399,8 +1398,7 @@ code = ''' goto dead; } int err = lfsr_file_sync(&lfs, &sim_files[j]->file); - assert(err == ((!sim_files[j]->zombie) ? 0 : LFS_ERR_NOENT) - || err == LFS_ERR_NOSPC); + assert(err == 0 || err == LFS_ERR_NOSPC); if (err == LFS_ERR_NOSPC) { goto dead; } diff --git a/tests/test_gc.toml b/tests/test_gc.toml index 9b2f9f10..92a1438a 100644 --- a/tests/test_gc.toml +++ b/tests/test_gc.toml @@ -2953,8 +2953,7 @@ code = ''' wbuf[k] = 'a' + (TEST_PRNG(&wprng_) % 26); } lfsr_file_write(&lfs, &sim_files[j]->file, wbuf, SIZE) => SIZE; - lfsr_file_sync(&lfs, &sim_files[j]->file) - => (!sim_files[j]->zombie) ? 0 : LFS_ERR_NOENT; + lfsr_file_sync(&lfs, &sim_files[j]->file) => 0; // update sim sim_files[j]->prng = wprng; @@ -3405,8 +3404,7 @@ code = ''' wbuf[k] = 'a' + (TEST_PRNG(&wprng_) % 26); } lfsr_file_write(&lfs, &sim_files[j]->file, wbuf, SIZE) => SIZE; - lfsr_file_sync(&lfs, &sim_files[j]->file) - => (!sim_files[j]->zombie) ? 0 : LFS_ERR_NOENT; + lfsr_file_sync(&lfs, &sim_files[j]->file) => 0; // update sim sim_files[j]->prng = wprng; diff --git a/tests/test_grow.toml b/tests/test_grow.toml index 4c2088b5..48faba57 100644 --- a/tests/test_grow.toml +++ b/tests/test_grow.toml @@ -1407,8 +1407,7 @@ code = ''' goto grow; } int err = lfsr_file_sync(&lfs, &sim_files[j]->file); - assert(err == ((!sim_files[j]->zombie) ? 0 : LFS_ERR_NOENT) - || err == LFS_ERR_NOSPC); + assert(err == 0 || err == LFS_ERR_NOSPC); if (err == LFS_ERR_NOSPC) { goto grow; } @@ -1915,8 +1914,7 @@ code = ''' goto grow; } int err = lfsr_file_sync(&lfs, &sim_files[j]->file); - assert(err == ((!sim_files[j]->zombie) ? 0 : LFS_ERR_NOENT) - || err == LFS_ERR_NOSPC); + assert(err == 0 || err == LFS_ERR_NOSPC); if (err == LFS_ERR_NOSPC) { goto grow; } diff --git a/tests/test_relocations.toml b/tests/test_relocations.toml index 89ca22a7..cdaf1dc9 100644 --- a/tests/test_relocations.toml +++ b/tests/test_relocations.toml @@ -667,8 +667,7 @@ code = ''' wbuf[k] = 'a' + (TEST_PRNG(&wprng_) % 26); } lfsr_file_write(&lfs, &sim_files[j]->file, wbuf, SIZE) => SIZE; - lfsr_file_sync(&lfs, &sim_files[j]->file) - => (!sim_files[j]->zombie) ? 0 : LFS_ERR_NOENT; + lfsr_file_sync(&lfs, &sim_files[j]->file) => 0; // update sim sim_files[j]->prng = wprng; @@ -1096,8 +1095,7 @@ code = ''' wbuf[k] = 'a' + (TEST_PRNG(&wprng_) % 26); } lfsr_file_write(&lfs, &sim_files[j]->file, wbuf, SIZE) => SIZE; - lfsr_file_sync(&lfs, &sim_files[j]->file) - => (!sim_files[j]->zombie) ? 0 : LFS_ERR_NOENT; + lfsr_file_sync(&lfs, &sim_files[j]->file) => 0; // update sim sim_files[j]->prng = wprng; diff --git a/tests/test_stickynotes.toml b/tests/test_stickynotes.toml index 7a24ea51..da79309b 100644 --- a/tests/test_stickynotes.toml +++ b/tests/test_stickynotes.toml @@ -5318,8 +5318,10 @@ code = ''' lfsr_file_open(&lfs, &file_, "batman", LFS_O_WRONLY) => LFS_ERR_NOENT; lfsr_file_open(&lfs, &file_, "batman", LFS_O_RDWR) => LFS_ERR_NOENT; - // removed files can not be synced - lfsr_file_sync(&lfs, &file) => LFS_ERR_NOENT; + // syncing removed files is a noop + lfsr_file_sync(&lfs, &file) => 0; + // resyncing removed files is an error + lfsr_file_resync(&lfs, &file) => LFS_ERR_NOENT; // but we should still be able to read our file handle lfsr_file_rewind(&lfs, &file) => 0; @@ -5451,8 +5453,10 @@ code = ''' lfsr_file_open(&lfs, &file_, "batman", LFS_O_WRONLY) => LFS_ERR_NOENT; lfsr_file_open(&lfs, &file_, "batman", LFS_O_RDWR) => LFS_ERR_NOENT; - // removed files can not be synced - lfsr_file_sync(&lfs, &file) => LFS_ERR_NOENT; + // syncing removed files is a noop + lfsr_file_sync(&lfs, &file) => 0; + // resyncing removed files is an error + lfsr_file_resync(&lfs, &file) => LFS_ERR_NOENT; // but we should still be able to read our file handle lfsr_file_rewind(&lfs, &file) => 0; @@ -5627,9 +5631,11 @@ code = ''' lfsr_file_read(&lfs, &file_, rbuf, CHUNK) => 0; lfsr_file_close(&lfs, &file_) => 0; - // removed files can not be synced - lfsr_file_sync(&lfs, &file) => LFS_ERR_NOENT; - // but we should be able to sync our second file just fine + // syncing removed files is a noop + lfsr_file_sync(&lfs, &file) => 0; + // resyncing removed files is an error + lfsr_file_resync(&lfs, &file) => LFS_ERR_NOENT; + // syncing our second file should actually sync lfsr_file_sync(&lfs, &file__) => 0; // second file should appear on disk now @@ -5886,9 +5892,11 @@ code = ''' lfsr_file_read(&lfs, &file_, rbuf, CHUNK) => 0; lfsr_file_close(&lfs, &file_) => 0; - // removed files can not be synced - lfsr_file_sync(&lfs, &file) => LFS_ERR_NOENT; - // but we should be able to sync our second file just fine + // syncing removed files is a noop + lfsr_file_sync(&lfs, &file) => 0; + // resyncing removed files is an error + lfsr_file_resync(&lfs, &file) => LFS_ERR_NOENT; + // syncing our second file should actually sync lfsr_file_sync(&lfs, &file__) => 0; // second file should appear on disk now @@ -6187,10 +6195,13 @@ code = ''' lfsr_file_read(&lfs, &file_, rbuf, CHUNK) => 0; lfsr_file_close(&lfs, &file_) => 0; - // removed files can not be synced - lfsr_file_sync(&lfs, &file) => LFS_ERR_NOENT; - lfsr_file_sync(&lfs, &file__) => LFS_ERR_NOENT; - // but we should be able to sync our third file just fine + // syncing removed files is a noop + lfsr_file_sync(&lfs, &file) => 0; + lfsr_file_sync(&lfs, &file__) => 0; + // resyncing removed files is an error + lfsr_file_resync(&lfs, &file) => LFS_ERR_NOENT; + lfsr_file_resync(&lfs, &file__) => LFS_ERR_NOENT; + // syncing our third file should actually sync lfsr_file_sync(&lfs, &file___) => 0; // third file should appear on disk now @@ -6502,10 +6513,13 @@ code = ''' lfsr_file_read(&lfs, &file_, rbuf, CHUNK) => 0; lfsr_file_close(&lfs, &file_) => 0; - // removed files can not be synced - lfsr_file_sync(&lfs, &file) => LFS_ERR_NOENT; - lfsr_file_sync(&lfs, &file__) => LFS_ERR_NOENT; - // but we should be able to sync our third file just fine + // syncing removed files is a noop + lfsr_file_sync(&lfs, &file) => 0; + lfsr_file_sync(&lfs, &file__) => 0; + // resyncing removed files is an error + lfsr_file_resync(&lfs, &file) => LFS_ERR_NOENT; + lfsr_file_resync(&lfs, &file__) => LFS_ERR_NOENT; + // syncing our third file should actually sync lfsr_file_sync(&lfs, &file___) => 0; // third file should appear on disk now @@ -10892,8 +10906,7 @@ code = ''' wbuf[k] = 'a' + (TEST_PRNG(&wprng_) % 26); } lfsr_file_write(&lfs, &sim_files[j]->file, wbuf, SIZE) => SIZE; - lfsr_file_sync(&lfs, &sim_files[j]->file) - => (!sim_files[j]->zombie) ? 0 : LFS_ERR_NOENT; + lfsr_file_sync(&lfs, &sim_files[j]->file) => 0; // update sim sim_files[j]->prng = wprng; @@ -11320,8 +11333,7 @@ code = ''' wbuf[k] = 'a' + (TEST_PRNG(&wprng_) % 26); } lfsr_file_write(&lfs, &sim_files[j]->file, wbuf, SIZE) => SIZE; - lfsr_file_sync(&lfs, &sim_files[j]->file) - => (!sim_files[j]->zombie) ? 0 : LFS_ERR_NOENT; + lfsr_file_sync(&lfs, &sim_files[j]->file) => 0; // update sim sim_files[j]->prng = wprng; diff --git a/tests/test_traversal.toml b/tests/test_traversal.toml index 94914356..27e43e5e 100644 --- a/tests/test_traversal.toml +++ b/tests/test_traversal.toml @@ -8629,8 +8629,7 @@ code = ''' wbuf[k] = 'a' + (TEST_PRNG(&wprng_) % 26); } lfsr_file_write(&lfs, &sim_files[j]->file, wbuf, SIZE) => SIZE; - lfsr_file_sync(&lfs, &sim_files[j]->file) - => (!sim_files[j]->zombie) ? 0 : LFS_ERR_NOENT; + lfsr_file_sync(&lfs, &sim_files[j]->file) => 0; // update sim sim_files[j]->prng = wprng; @@ -9087,8 +9086,7 @@ code = ''' wbuf[k] = 'a' + (TEST_PRNG(&wprng_) % 26); } lfsr_file_write(&lfs, &sim_files[j]->file, wbuf, SIZE) => SIZE; - lfsr_file_sync(&lfs, &sim_files[j]->file) - => (!sim_files[j]->zombie) ? 0 : LFS_ERR_NOENT; + lfsr_file_sync(&lfs, &sim_files[j]->file) => 0; // update sim sim_files[j]->prng = wprng;