From 4bd653dd006ec4568ebae6d710f30ff5d853ed8e Mon Sep 17 00:00:00 2001 From: Noah Gorny Date: Wed, 17 Jun 2020 15:42:46 +0300 Subject: [PATCH 1/3] Assert that file/dir struct is not reused in lfs_file_opencfg/lfs_dir_open --- lfs.c | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/lfs.c b/lfs.c index eb832fa..2b179a6 100644 --- a/lfs.c +++ b/lfs.c @@ -411,6 +411,17 @@ static inline void lfs_superblock_tole32(lfs_superblock_t *superblock) { superblock->attr_max = lfs_tole32(superblock->attr_max); } +static inline bool lfs_mlist_isopen(struct lfs_mlist *head, + struct lfs_mlist *node) { + for (struct lfs_mlist **p = &head; *p; p = &(*p)->next) { + if (*p == (struct lfs_mlist*)node) { + return true; + } + } + + return false; +} + /// Internal operations predeclared here /// static int lfs_dir_commit(lfs_t *lfs, lfs_mdir_t *dir, @@ -2007,6 +2018,8 @@ int lfs_mkdir(lfs_t *lfs, const char *path) { int lfs_dir_open(lfs_t *lfs, lfs_dir_t *dir, const char *path) { LFS_TRACE("lfs_dir_open(%p, %p, \"%s\")", (void*)lfs, (void*)dir, path); + LFS_ASSERT(!lfs_mlist_isopen(lfs->mlist, (struct lfs_mlist*)dir)); + lfs_stag_t tag = lfs_dir_find(lfs, &dir->m, &path, NULL); if (tag < 0) { LFS_TRACE("lfs_dir_open -> %"PRId32, tag); @@ -2387,6 +2400,7 @@ int lfs_file_opencfg(lfs_t *lfs, lfs_file_t *file, ".buffer=%p, .attrs=%p, .attr_count=%"PRIu32"})", (void*)lfs, (void*)file, path, flags, (void*)cfg, cfg->buffer, (void*)cfg->attrs, cfg->attr_count); + LFS_ASSERT(!lfs_mlist_isopen(lfs->mlist, (struct lfs_mlist*)file)); // deorphan if we haven't yet, needed at most once after poweron if ((flags & 3) != LFS_O_RDONLY) { From 6303558aee5c7ab9a5eb3ac495b3094647e962b4 Mon Sep 17 00:00:00 2001 From: Noah Gorny Date: Wed, 17 Jun 2020 20:46:11 +0300 Subject: [PATCH 2/3] Use LFS_O_RDWR instead of magic number in lfs_file_* asserts --- lfs.c | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/lfs.c b/lfs.c index 2b179a6..4b9fa6f 100644 --- a/lfs.c +++ b/lfs.c @@ -2403,7 +2403,7 @@ int lfs_file_opencfg(lfs_t *lfs, lfs_file_t *file, LFS_ASSERT(!lfs_mlist_isopen(lfs->mlist, (struct lfs_mlist*)file)); // deorphan if we haven't yet, needed at most once after poweron - if ((flags & 3) != LFS_O_RDONLY) { + if ((flags & LFS_O_RDWR) != LFS_O_RDONLY) { int err = lfs_fs_forceconsistency(lfs); if (err) { LFS_TRACE("lfs_file_opencfg -> %d", err); @@ -2478,7 +2478,7 @@ int lfs_file_opencfg(lfs_t *lfs, lfs_file_t *file, // fetch attrs for (unsigned i = 0; i < file->cfg->attr_count; i++) { - if ((file->flags & 3) != LFS_O_WRONLY) { + if ((file->flags & LFS_O_RDWR) != LFS_O_WRONLY) { lfs_stag_t res = lfs_dir_get(lfs, &file->m, LFS_MKTAG(0x7ff, 0x3ff, 0), LFS_MKTAG(LFS_TYPE_USERATTR + file->cfg->attrs[i].type, @@ -2490,7 +2490,7 @@ int lfs_file_opencfg(lfs_t *lfs, lfs_file_t *file, } } - if ((file->flags & 3) != LFS_O_RDONLY) { + if ((file->flags & LFS_O_RDWR) != LFS_O_RDONLY) { if (file->cfg->attrs[i].size > lfs->attr_max) { err = LFS_ERR_NOSPC; goto cleanup; @@ -2807,7 +2807,7 @@ lfs_ssize_t lfs_file_read(lfs_t *lfs, lfs_file_t *file, LFS_TRACE("lfs_file_read(%p, %p, %p, %"PRIu32")", (void*)lfs, (void*)file, buffer, size); LFS_ASSERT(file->flags & LFS_F_OPENED); - LFS_ASSERT((file->flags & 3) != LFS_O_WRONLY); + LFS_ASSERT((file->flags & LFS_O_RDWR) != LFS_O_WRONLY); uint8_t *data = buffer; lfs_size_t nsize = size; @@ -2887,7 +2887,7 @@ lfs_ssize_t lfs_file_write(lfs_t *lfs, lfs_file_t *file, LFS_TRACE("lfs_file_write(%p, %p, %p, %"PRIu32")", (void*)lfs, (void*)file, buffer, size); LFS_ASSERT(file->flags & LFS_F_OPENED); - LFS_ASSERT((file->flags & 3) != LFS_O_RDONLY); + LFS_ASSERT((file->flags & LFS_O_RDWR) != LFS_O_RDONLY); const uint8_t *data = buffer; lfs_size_t nsize = size; @@ -3052,7 +3052,7 @@ int lfs_file_truncate(lfs_t *lfs, lfs_file_t *file, lfs_off_t size) { LFS_TRACE("lfs_file_truncate(%p, %p, %"PRIu32")", (void*)lfs, (void*)file, size); LFS_ASSERT(file->flags & LFS_F_OPENED); - LFS_ASSERT((file->flags & 3) != LFS_O_RDONLY); + LFS_ASSERT((file->flags & LFS_O_RDWR) != LFS_O_RDONLY); if (size > LFS_FILE_MAX) { LFS_TRACE("lfs_file_truncate -> %d", LFS_ERR_INVAL); From 008ebc37dfe09c67b7b8b3b511f44399d3c3c684 Mon Sep 17 00:00:00 2001 From: Noah Gorny Date: Wed, 18 Nov 2020 00:20:34 +0200 Subject: [PATCH 3/3] Add lfs_mlist_append/remove helper --- lfs.c | 34 ++++++++++++++++++---------------- 1 file changed, 18 insertions(+), 16 deletions(-) diff --git a/lfs.c b/lfs.c index 4b9fa6f..d20f6f1 100644 --- a/lfs.c +++ b/lfs.c @@ -422,6 +422,20 @@ static inline bool lfs_mlist_isopen(struct lfs_mlist *head, return false; } +static inline void lfs_mlist_remove(lfs_t *lfs, struct lfs_mlist *mlist) { + for (struct lfs_mlist **p = &lfs->mlist; *p; p = &(*p)->next) { + if (*p == mlist) { + *p = (*p)->next; + break; + } + } +} + +static inline void lfs_mlist_append(lfs_t *lfs, struct lfs_mlist *mlist) { + mlist->next = lfs->mlist; + lfs->mlist = mlist; +} + /// Internal operations predeclared here /// static int lfs_dir_commit(lfs_t *lfs, lfs_mdir_t *dir, @@ -2062,8 +2076,7 @@ int lfs_dir_open(lfs_t *lfs, lfs_dir_t *dir, const char *path) { // add to list of mdirs dir->type = LFS_TYPE_DIR; - dir->next = (lfs_dir_t*)lfs->mlist; - lfs->mlist = (struct lfs_mlist*)dir; + lfs_mlist_append(lfs, (struct lfs_mlist *)dir); LFS_TRACE("lfs_dir_open -> %d", 0); return 0; @@ -2072,12 +2085,7 @@ int lfs_dir_open(lfs_t *lfs, lfs_dir_t *dir, const char *path) { int lfs_dir_close(lfs_t *lfs, lfs_dir_t *dir) { LFS_TRACE("lfs_dir_close(%p, %p)", (void*)lfs, (void*)dir); // remove from list of mdirs - for (struct lfs_mlist **p = &lfs->mlist; *p; p = &(*p)->next) { - if (*p == (struct lfs_mlist*)dir) { - *p = (*p)->next; - break; - } - } + lfs_mlist_remove(lfs, (struct lfs_mlist *)dir); LFS_TRACE("lfs_dir_close -> %d", 0); return 0; @@ -2428,8 +2436,7 @@ int lfs_file_opencfg(lfs_t *lfs, lfs_file_t *file, // get id, add to list of mdirs to catch update changes file->type = LFS_TYPE_REG; - file->next = (lfs_file_t*)lfs->mlist; - lfs->mlist = (struct lfs_mlist*)file; + lfs_mlist_append(lfs, (struct lfs_mlist *)file); if (tag == LFS_ERR_NOENT) { if (!(flags & LFS_O_CREAT)) { @@ -2565,12 +2572,7 @@ int lfs_file_close(lfs_t *lfs, lfs_file_t *file) { int err = lfs_file_sync(lfs, file); // remove from list of mdirs - for (struct lfs_mlist **p = &lfs->mlist; *p; p = &(*p)->next) { - if (*p == (struct lfs_mlist*)file) { - *p = (*p)->next; - break; - } - } + lfs_mlist_remove(lfs, (struct lfs_mlist*)file); // clean up memory if (!file->cfg->buffer) {