diff --git a/lfs.c b/lfs.c index ec94fdc7..ac46d141 100644 --- a/lfs.c +++ b/lfs.c @@ -10659,11 +10659,12 @@ failed:; } int lfsr_file_flush(lfs_t *lfs, lfsr_file_t *file) { - // flushing readonly files is not supported - // - // in theory we could make this a noop, but that would be inconsistent - // with lfsr_file_sync - LFS_ASSERT(lfsr_o_iswriteable(file->flags)); + // readonly files should do nothing + LFS_ASSERT(lfsr_o_iswriteable(file->flags) + || !lfsr_f_isunflushed(file->flags) + || (lfsr_file_size_(file) <= lfs->cfg->cache_size + && lfsr_file_size_(file) <= lfs->cfg->inline_size + && lfsr_file_size_(file) <= lfs->cfg->fragment_size)); // do nothing if our file is already flushed if (!lfsr_f_isunflushed(file->flags)) { @@ -10707,12 +10708,6 @@ failed:; } int lfsr_file_sync(lfs_t *lfs, lfsr_file_t *file) { - // syncing readonly files is not supported - // - // in theory we could make this a noop, but then syncing desynced - // readonly files would require disk writes - LFS_ASSERT(lfsr_o_iswriteable(file->flags)); - // do nothing if our file has been removed if (file->mdir.mid == -1) { return 0; @@ -10751,6 +10746,21 @@ int lfsr_file_sync(lfs_t *lfs, lfsr_file_t *file) { // don't write to disk if our disk is already in-sync if (lfsr_f_isunsynced(file->flags)) { + // readonly files should do nothing + // + // but readonly files _can_ end up unsynced, in the roundabout + // case where: + // + // 1. a file is opened rdonly + desync + // 2. the same file is opened and written to + // 3. we try to sync our original file handle + // + // the best thing we can do in this case is return an error + if (!lfsr_o_iswriteable(file->flags)) { + err = LFS_ERR_INVAL; + goto failed; + } + // checkpoint the allocator again lfs_alloc_ckpoint(lfs); diff --git a/tests/test_fsync.toml b/tests/test_fsync.toml index b6e583b1..60fa2e46 100644 --- a/tests/test_fsync.toml +++ b/tests/test_fsync.toml @@ -2085,6 +2085,107 @@ code = ''' lfsr_unmount(&lfs) => 0; ''' +[cases.test_fsync_desync_wrrd_noop] +code = ''' + lfs_t lfs; + lfsr_format(&lfs, CFG) => 0; + lfsr_mount(&lfs, CFG) => 0; + + // a - writer + // b - reader kept open, recvs updates from a + // c - desynced reader + lfsr_file_t a; + lfsr_file_t b; + lfsr_file_t c; + uint8_t rbuf[256]; + lfsr_file_open(&lfs, &a, "jello", + LFS_O_WRONLY | LFS_O_CREAT | LFS_O_EXCL) => 0; + lfsr_file_open(&lfs, &b, "jello", LFS_O_RDONLY) => 0; + lfsr_file_open(&lfs, &c, "jello", LFS_O_RDONLY | LFS_O_DESYNC) => 0; + + // write to a and sync + lfsr_file_write(&lfs, &a, "hello!", strlen("hello!")) + => strlen("hello!"); + lfsr_file_sync(&lfs, &a) => 0; + + // our write should show up in b + lfsr_file_read(&lfs, &b, rbuf, sizeof(rbuf)) => strlen("hello!"); + assert(memcmp(rbuf, "hello!", strlen("hello!")) == 0); + // but not in c + lfsr_file_read(&lfs, &c, rbuf, sizeof(rbuf)) => 0; + // reopen c, should now be up to date + lfsr_file_close(&lfs, &c) => 0; + lfsr_file_open(&lfs, &c, "jello", LFS_O_RDONLY | LFS_O_DESYNC) => 0; + lfsr_file_read(&lfs, &c, rbuf, sizeof(rbuf)) => strlen("hello!"); + assert(memcmp(rbuf, "hello!", strlen("hello!")) == 0); + + // rewrite a and sync + lfsr_file_rewind(&lfs, &a) => 0; + lfsr_file_write(&lfs, &a, "bonjour!", strlen("bonjour!")) + => strlen("bonjour!"); + lfsr_file_sync(&lfs, &a) => 0; + + // our write should show up in b + lfsr_file_rewind(&lfs, &b) => 0; + lfsr_file_read(&lfs, &b, rbuf, sizeof(rbuf)) => strlen("bonjour!"); + assert(memcmp(rbuf, "bonjour!", strlen("bonjour!")) == 0); + // but not in c + lfsr_file_rewind(&lfs, &c) => 0; + lfsr_file_read(&lfs, &c, rbuf, sizeof(rbuf)) => strlen("hello!"); + assert(memcmp(rbuf, "hello!", strlen("hello!")) == 0); + + // sync b, this should be a noop + lfsr_file_sync(&lfs, &b) => 0; + + // our write should show up in b + lfsr_file_rewind(&lfs, &b) => 0; + lfsr_file_read(&lfs, &b, rbuf, sizeof(rbuf)) => strlen("bonjour!"); + assert(memcmp(rbuf, "bonjour!", strlen("bonjour!")) == 0); + // but not in c + lfsr_file_rewind(&lfs, &c) => 0; + lfsr_file_read(&lfs, &c, rbuf, sizeof(rbuf)) => strlen("hello!"); + assert(memcmp(rbuf, "hello!", strlen("hello!")) == 0); + + // as if things couldn't get weirder + // + // what do you think should happend if we sync c? + // + // in theory, this should update a + b + disk, but that would require + // writing to disk... instead we just error + lfsr_file_sync(&lfs, &c) => LFS_ERR_INVAL; + + // our write should show up in b + lfsr_file_rewind(&lfs, &b) => 0; + lfsr_file_read(&lfs, &b, rbuf, sizeof(rbuf)) => strlen("bonjour!"); + assert(memcmp(rbuf, "bonjour!", strlen("bonjour!")) == 0); + // but not in c + lfsr_file_rewind(&lfs, &c) => 0; + lfsr_file_read(&lfs, &c, rbuf, sizeof(rbuf)) => strlen("hello!"); + assert(memcmp(rbuf, "hello!", strlen("hello!")) == 0); + // reopen c, should now be up to date + lfsr_file_close(&lfs, &c) => 0; + lfsr_file_open(&lfs, &c, "jello", LFS_O_RDONLY | LFS_O_DESYNC) => 0; + lfsr_file_read(&lfs, &c, rbuf, sizeof(rbuf)) => strlen("bonjour!"); + assert(memcmp(rbuf, "bonjour!", strlen("bonjour!")) == 0); + + // we _are_ allowed to sync c if it results in a noop + lfsr_file_sync(&lfs, &c) => 0; + + // our write should show up in b + lfsr_file_rewind(&lfs, &b) => 0; + lfsr_file_read(&lfs, &b, rbuf, sizeof(rbuf)) => strlen("bonjour!"); + assert(memcmp(rbuf, "bonjour!", strlen("bonjour!")) == 0); + // and in c + lfsr_file_rewind(&lfs, &c) => 0; + lfsr_file_read(&lfs, &c, rbuf, sizeof(rbuf)) => strlen("bonjour!"); + assert(memcmp(rbuf, "bonjour!", strlen("bonjour!")) == 0); + + lfsr_file_close(&lfs, &a) => 0; + lfsr_file_close(&lfs, &b) => 0; + lfsr_file_close(&lfs, &c) => 0; + lfsr_unmount(&lfs) => 0; +''' + [cases.test_fsync_desync_wdwdrr_append] code = ''' lfs_t lfs;