diff --git a/lfs.c b/lfs.c index 92276b8c..88089b87 100644 --- a/lfs.c +++ b/lfs.c @@ -11346,12 +11346,17 @@ static inline lfs_off_t lfsr_file_size_(const lfsr_file_t *file) { static lfs_ssize_t lfsr_file_read_(lfs_t *lfs, const lfsr_file_t *file, lfs_off_t pos, uint8_t *buffer, lfs_size_t size); -static int lfsr_file_fetch(lfs_t *lfs, lfsr_file_t *file) { +static int lfsr_file_fetch(lfs_t *lfs, lfsr_file_t *file, bool trunc) { // default data state - file->o.bshrub_ = LFSR_BSHRUB_BNULL(); + file->o.bshrub = LFSR_BSHRUB_BNULL(); + // discard the current buffer + file->buffer.pos = 0; + file->buffer.size = 0; + // mark as flushed + file->o.o.flags &= ~LFS_O_UNFLUSH; - // don't bother reading disk if we're an orphan - if (!lfsr_o_isorphan(file->o.o.flags)) { + // don't bother reading disk if we're an orphan or truncating + if (!lfsr_o_isorphan(file->o.o.flags) && !trunc) { // lookup the file struct, if there is one lfsr_tag_t tag; lfsr_data_t data; @@ -11364,6 +11369,7 @@ static int lfsr_file_fetch(lfs_t *lfs, lfsr_file_t *file) { // note many of these functions leave bshrub undefined if // there is an error, so we first read into the staging // bshrub + file->o.bshrub_ = file->o.bshrub; // may be a sprout (simple inlined data) if (err != LFS_ERR_NOENT && tag == LFSR_TAG_DATA) { @@ -11393,19 +11399,13 @@ static int lfsr_file_fetch(lfs_t *lfs, lfsr_file_t *file) { return err; } } - } - // mark as flushed and synced if we're not an orphan - file->o.o.flags &= ~( - LFS_O_UNFLUSH - | ((!lfsr_o_isorphan(file->o.o.flags)) - ? LFS_O_UNSYNC - : 0)); - // update the bshrub - file->o.bshrub = file->o.bshrub_; - // discard the current buffer - file->buffer.pos = 0; - file->buffer.size = 0; + // update the bshrub + file->o.bshrub = file->o.bshrub_; + + // mark as synced + file->o.o.flags &= ~LFS_O_UNSYNC; + } // if our file is small, try to keep the whole thing in our buffer // @@ -11433,6 +11433,14 @@ static int lfsr_file_fetch(lfs_t *lfs, lfsr_file_t *file) { continue; } + // don't bother reading disk if we're an orphan + if (lfsr_o_isorphan(file->o.o.flags)) { + if (file->cfg->attrs[i].size) { + *file->cfg->attrs[i].size = LFS_ERR_NOATTR; + } + continue; + } + // lookup the attr lfsr_data_t data; int err = lfsr_mdir_lookup(lfs, &file->o.o.mdir, @@ -11467,6 +11475,7 @@ static int lfsr_file_fetch(lfs_t *lfs, lfsr_file_t *file) { } // needed in lfsr_file_opencfg +static void lfsr_file_close_(lfs_t *lfs, const lfsr_file_t *file); static int lfsr_file_ck(lfs_t *lfs, const lfsr_file_t *file, uint32_t flags); @@ -11518,8 +11527,6 @@ int lfsr_file_opencfg(lfs_t *lfs, lfsr_file_t *file, | (lfs->flags & (LFS_M_FLUSH | LFS_M_SYNC)) // default to unflushed for orphans/truncated files | LFS_O_UNFLUSH; - // default data state - file->o.bshrub = LFSR_BSHRUB_BNULL(); file->pos = 0; file->eblock = 0; file->eoff = -1; @@ -11599,12 +11606,11 @@ int lfsr_file_opencfg(lfs_t *lfs, lfsr_file_t *file, file->buffer.pos = 0; file->buffer.size = 0; - // fetch the file struct if we're not truncating - if (!lfsr_o_istrunc(file->o.o.flags)) { - err = lfsr_file_fetch(lfs, file); - if (err) { - goto failed; - } + // fetch the file struct and custom attrs + err = lfsr_file_fetch(lfs, file, + lfsr_o_istrunc(file->o.o.flags)); + if (err) { + goto failed; } // check metadata/data for errors? @@ -11620,11 +11626,8 @@ int lfsr_file_opencfg(lfs_t *lfs, lfsr_file_t *file, return 0; failed:; - // clean up memory - if (!file->cfg->buffer) { - lfs_free(file->buffer.buffer); - } - + // clean up resources + lfsr_file_close_(lfs, file); return err; } @@ -11636,22 +11639,8 @@ int lfsr_file_open(lfs_t *lfs, lfsr_file_t *file, return lfsr_file_opencfg(lfs, file, path, flags, &lfsr_file_defaults); } -// needed in lfsr_file_close -int lfsr_file_sync(lfs_t *lfs, lfsr_file_t *file); - -int lfsr_file_close(lfs_t *lfs, lfsr_file_t *file) { - LFS_ASSERT(lfsr_omdir_isopen(lfs, &file->o.o)); - - // don't call lfsr_file_sync if we're readonly or desynced - int err = 0; - if (!lfsr_o_isrdonly(file->o.o.flags) - && !lfsr_o_isdesync(file->o.o.flags)) { - err = lfsr_file_sync(lfs, file); - } - - // remove from tracked mdirs - lfsr_omdir_close(lfs, &file->o.o); - +// clean up resources +static void lfsr_file_close_(lfs_t *lfs, const lfsr_file_t *file) { // clean up memory if (!file->cfg->buffer) { lfs_free(file->buffer.buffer); @@ -11675,6 +11664,26 @@ int lfsr_file_close(lfs_t *lfs, lfsr_file_t *file) { lfs->flags |= LFS_I_HASORPHANS; } } +} + +// needed in lfsr_file_close +int lfsr_file_sync(lfs_t *lfs, lfsr_file_t *file); + +int lfsr_file_close(lfs_t *lfs, lfsr_file_t *file) { + LFS_ASSERT(lfsr_omdir_isopen(lfs, &file->o.o)); + + // don't call lfsr_file_sync if we're readonly or desynced + int err = 0; + if (!lfsr_o_isrdonly(file->o.o.flags) + && !lfsr_o_isdesync(file->o.o.flags)) { + err = lfsr_file_sync(lfs, file); + } + + // remove from tracked mdirs + lfsr_omdir_close(lfs, &file->o.o); + + // clean up resources + lfsr_file_close_(lfs, file); return err; } @@ -13202,7 +13211,7 @@ int lfsr_file_resync(lfs_t *lfs, lfsr_file_t *file) { // do nothing if already in-sync if (lfsr_o_isunsync(file->o.o.flags)) { // refetch the file struct from disk - err = lfsr_file_fetch(lfs, file); + err = lfsr_file_fetch(lfs, file, false); if (err) { goto failed; } diff --git a/tests/test_attrs.toml b/tests/test_attrs.toml index 3331fe22..0a907c74 100644 --- a/tests/test_attrs.toml +++ b/tests/test_attrs.toml @@ -1611,6 +1611,148 @@ code = ''' lfsr_unmount(&lfs) => 0; ''' +# catch LFS_O_TRUNC mistakes, this has created issues before +[cases.test_attrs_fattr_otrunc] +defines.MODE = ['LFS_A_RDWR'] +defines.MUTSIZE = [false, true] +code = ''' + lfs_t lfs; + lfsr_format(&lfs, LFS_F_RDWR, CFG) => 0; + lfsr_mount(&lfs, LFS_M_RDWR, CFG) => 0; + + // create a file + lfsr_file_t file; + lfsr_file_open(&lfs, &file, "cat", LFS_O_WRONLY | LFS_O_CREAT) => 0; + lfsr_file_write(&lfs, &file, "meow", strlen("meow")) => strlen("meow"); + lfsr_file_close(&lfs, &file) => 0; + + // create some attrs + const char *a = "One 18.25 ounce package chocolate cake mix."; + lfsr_setattr(&lfs, "cat", 'a', a, strlen(a)) => 0; + const char *b = "One can prepared coconut pecan frosting."; + lfsr_setattr(&lfs, "cat", 'b', b, strlen(b)) => 0; + const char *c = "Three slash four cup vegetable oil."; + lfsr_setattr(&lfs, "cat", 'c', c, strlen(c)) => 0; + + // try opening a file with these attrs + uint8_t a_buf[256]; + lfs_ssize_t a_size = -1; + uint8_t b_buf[256]; + lfs_ssize_t b_size = -1; + uint8_t c_buf[256]; + lfs_ssize_t c_size = -1; + struct lfs_attr attrs[] = { + { + .type = 'a', + .flags = MODE, + .buffer = a_buf, + .buffer_size = sizeof(a_buf), + .size = (MUTSIZE) ? &a_size : NULL, + }, + { + .type = 'b', + .flags = MODE, + .buffer = b_buf, + .buffer_size = sizeof(b_buf), + .size = (MUTSIZE) ? &b_size : NULL, + }, + { + .type = 'c', + .flags = MODE, + .buffer = c_buf, + .buffer_size = sizeof(c_buf), + .size = (MUTSIZE) ? &c_size : NULL, + } + }; + struct lfs_file_config filecfg = { + .attrs = attrs, + .attr_count = 3, + }; + lfsr_file_opencfg(&lfs, &file, "cat", + MODE | LFS_O_TRUNC, &filecfg) => 0; + + // did we read the attrs correctly? + if (MUTSIZE) { + assert(a_size == strlen(a)); + } + assert(memcmp(a_buf, a, strlen(a)) == 0); + if (MUTSIZE) { + assert(b_size == strlen(b)); + } + assert(memcmp(b_buf, b, strlen(b)) == 0); + if (MUTSIZE) { + assert(c_size == strlen(c)); + } + assert(memcmp(c_buf, c, strlen(c)) == 0); + + lfsr_file_close(&lfs, &file) => 0; + lfsr_unmount(&lfs) => 0; +''' + +# orphan files should zero attributes +[cases.test_attrs_fattr_orphan] +defines.MODE = ['LFS_A_RDWR'] +defines.MUTSIZE = [false, true] +code = ''' + lfs_t lfs; + lfsr_format(&lfs, LFS_F_RDWR, CFG) => 0; + lfsr_mount(&lfs, LFS_M_RDWR, CFG) => 0; + + // _don't_ create a file + + // try opening a file with these attrs + uint8_t a_buf[256]; + lfs_ssize_t a_size = -1; + uint8_t b_buf[256]; + lfs_ssize_t b_size = -1; + uint8_t c_buf[256]; + lfs_ssize_t c_size = -1; + struct lfs_attr attrs[] = { + { + .type = 'a', + .flags = MODE, + .buffer = a_buf, + .buffer_size = sizeof(a_buf), + .size = (MUTSIZE) ? &a_size : NULL, + }, + { + .type = 'b', + .flags = MODE, + .buffer = b_buf, + .buffer_size = sizeof(b_buf), + .size = (MUTSIZE) ? &b_size : NULL, + }, + { + .type = 'c', + .flags = MODE, + .buffer = c_buf, + .buffer_size = sizeof(c_buf), + .size = (MUTSIZE) ? &c_size : NULL, + } + }; + struct lfs_file_config filecfg = { + .attrs = attrs, + .attr_count = 3, + }; + lfsr_file_t file; + lfsr_file_opencfg(&lfs, &file, "cat", + MODE | LFS_O_CREAT, &filecfg) => 0; + + // did we read the attrs correctly? + if (MUTSIZE) { + assert(a_size == LFS_ERR_NOATTR); + } + if (MUTSIZE) { + assert(b_size == LFS_ERR_NOATTR); + } + if (MUTSIZE) { + assert(c_size == LFS_ERR_NOATTR); + } + + lfsr_file_close(&lfs, &file) => 0; + lfsr_unmount(&lfs) => 0; +''' + # test that file-attached attrs are written correctly [cases.test_attrs_fattr_set] defines.MODE = ['LFS_A_WRONLY', 'LFS_A_RDWR'] @@ -2378,6 +2520,7 @@ code = ''' #[cases.test_attrs_fattr_zombie_resync] TODO do we test file zombie resync? #[cases.test_attrs_fattr_pl] ? +#[cases.test_attrs_fattr_orphan] ?