From 97d8d5e96a7781596708664f18f2ea6c3a179330 Mon Sep 17 00:00:00 2001 From: Christopher Haster Date: Sun, 7 Oct 2018 13:47:29 -0500 Subject: [PATCH] Fixed issue where a rename causes a split and pushes dir out of sync The issue happens when a rename causes a split in the destination pair. If the destination pair is the same as the source pair, this triggers the logic to keep both pairs in sync. Unfortunately, this logic didn't work, because the source entry still resides in the old source pair, unlike the destination pair, which is now in the new pair created by the split. The best fix for now is to refetch the source pair after the changes to the destination pair. This isn't the most efficient solution, but fortunately this bug has already been fixed in the revamped move logic in littlefs v2 (currently in progress). Found by ohoc --- lfs.c | 39 +++++++++++++--------------- lfs.h | 1 + tests/test_dirs.sh | 63 ++++++++++++++++++++++++++++++++++++++++++++-- 3 files changed, 80 insertions(+), 23 deletions(-) diff --git a/lfs.c b/lfs.c index d6b45f17..438647e4 100644 --- a/lfs.c +++ b/lfs.c @@ -888,7 +888,7 @@ static int lfs_dir_find(lfs_t *lfs, lfs_dir_t *dir, } // check that entry has not been moved - if (entry->d.type & 0x80) { + if (!lfs->moving && entry->d.type & 0x80) { int moved = lfs_moved(lfs, &entry->d.u); if (moved < 0 || moved) { return (moved < 0) ? moved : LFS_ERR_NOENT; @@ -1922,7 +1922,14 @@ int lfs_rename(lfs_t *lfs, const char *oldpath, const char *newpath) { // find old entry lfs_dir_t oldcwd; lfs_entry_t oldentry; - int err = lfs_dir_find(lfs, &oldcwd, &oldentry, &oldpath); + int err = lfs_dir_find(lfs, &oldcwd, &oldentry, &(const char *){oldpath}); + if (err) { + return err; + } + + // mark as moving + oldentry.d.type |= 0x80; + err = lfs_dir_update(lfs, &oldcwd, &oldentry, NULL); if (err) { return err; } @@ -1935,11 +1942,9 @@ int lfs_rename(lfs_t *lfs, const char *oldpath, const char *newpath) { return err; } - bool prevexists = (err != LFS_ERR_NOENT); - bool samepair = (lfs_paircmp(oldcwd.pair, newcwd.pair) == 0); - // must have same type - if (prevexists && preventry.d.type != oldentry.d.type) { + bool prevexists = (err != LFS_ERR_NOENT); + if (prevexists && preventry.d.type != (0x7f & oldentry.d.type)) { return LFS_ERR_ISDIR; } @@ -1956,18 +1961,6 @@ int lfs_rename(lfs_t *lfs, const char *oldpath, const char *newpath) { } } - // mark as moving - oldentry.d.type |= 0x80; - err = lfs_dir_update(lfs, &oldcwd, &oldentry, NULL); - if (err) { - return err; - } - - // update pair if newcwd == oldcwd - if (samepair) { - newcwd = oldcwd; - } - // move to new location lfs_entry_t newentry = preventry; newentry.d = oldentry.d; @@ -1986,10 +1979,13 @@ int lfs_rename(lfs_t *lfs, const char *oldpath, const char *newpath) { } } - // update pair if newcwd == oldcwd - if (samepair) { - oldcwd = newcwd; + // fetch old pair again in case dir block changed + lfs->moving = true; + err = lfs_dir_find(lfs, &oldcwd, &oldentry, &oldpath); + if (err) { + return err; } + lfs->moving = false; // remove old entry err = lfs_dir_remove(lfs, &oldcwd, &oldentry); @@ -2087,6 +2083,7 @@ static int lfs_init(lfs_t *lfs, const struct lfs_config *cfg) { lfs->files = NULL; lfs->dirs = NULL; lfs->deorphaned = false; + lfs->moving = false; return 0; diff --git a/lfs.h b/lfs.h index 7dd36046..4b687f4b 100644 --- a/lfs.h +++ b/lfs.h @@ -280,6 +280,7 @@ typedef struct lfs { lfs_free_t free; bool deorphaned; + bool moving; } lfs_t; diff --git a/tests/test_dirs.sh b/tests/test_dirs.sh index 53d76f7a..874808d4 100755 --- a/tests/test_dirs.sh +++ b/tests/test_dirs.sh @@ -326,13 +326,42 @@ tests/test.py << TEST lfs_unmount(&lfs) => 0; TEST +echo "--- Multi-block rename ---" +tests/test.py << TEST + lfs_mount(&lfs, &cfg) => 0; + for (int i = 0; i < $LARGESIZE; i++) { + sprintf((char*)buffer, "cactus/test%d", i); + sprintf((char*)wbuffer, "cactus/tedd%d", i); + lfs_rename(&lfs, (char*)buffer, (char*)wbuffer) => 0; + } + lfs_unmount(&lfs) => 0; +TEST +tests/test.py << TEST + lfs_mount(&lfs, &cfg) => 0; + lfs_dir_open(&lfs, &dir[0], "cactus") => 0; + lfs_dir_read(&lfs, &dir[0], &info) => 1; + strcmp(info.name, ".") => 0; + info.type => LFS_TYPE_DIR; + lfs_dir_read(&lfs, &dir[0], &info) => 1; + strcmp(info.name, "..") => 0; + info.type => LFS_TYPE_DIR; + for (int i = 0; i < $LARGESIZE; i++) { + sprintf((char*)buffer, "tedd%d", i); + lfs_dir_read(&lfs, &dir[0], &info) => 1; + strcmp(info.name, (char*)buffer) => 0; + info.type => LFS_TYPE_DIR; + } + lfs_dir_read(&lfs, &dir[0], &info) => 0; + lfs_unmount(&lfs) => 0; +TEST + echo "--- Multi-block remove ---" tests/test.py << TEST lfs_mount(&lfs, &cfg) => 0; lfs_remove(&lfs, "cactus") => LFS_ERR_NOTEMPTY; for (int i = 0; i < $LARGESIZE; i++) { - sprintf((char*)buffer, "cactus/test%d", i); + sprintf((char*)buffer, "cactus/tedd%d", i); lfs_remove(&lfs, (char*)buffer) => 0; } @@ -391,13 +420,43 @@ tests/test.py << TEST lfs_unmount(&lfs) => 0; TEST +echo "--- Multi-block rename with files ---" +tests/test.py << TEST + lfs_mount(&lfs, &cfg) => 0; + for (int i = 0; i < $LARGESIZE; i++) { + sprintf((char*)buffer, "prickly-pear/test%d", i); + sprintf((char*)wbuffer, "prickly-pear/tedd%d", i); + lfs_rename(&lfs, (char*)buffer, (char*)wbuffer) => 0; + } + lfs_unmount(&lfs) => 0; +TEST +tests/test.py << TEST + lfs_mount(&lfs, &cfg) => 0; + lfs_dir_open(&lfs, &dir[0], "prickly-pear") => 0; + lfs_dir_read(&lfs, &dir[0], &info) => 1; + strcmp(info.name, ".") => 0; + info.type => LFS_TYPE_DIR; + lfs_dir_read(&lfs, &dir[0], &info) => 1; + strcmp(info.name, "..") => 0; + info.type => LFS_TYPE_DIR; + for (int i = 0; i < $LARGESIZE; i++) { + sprintf((char*)buffer, "tedd%d", i); + lfs_dir_read(&lfs, &dir[0], &info) => 1; + strcmp(info.name, (char*)buffer) => 0; + info.type => LFS_TYPE_REG; + info.size => 6; + } + lfs_dir_read(&lfs, &dir[0], &info) => 0; + lfs_unmount(&lfs) => 0; +TEST + echo "--- Multi-block remove with files ---" tests/test.py << TEST lfs_mount(&lfs, &cfg) => 0; lfs_remove(&lfs, "prickly-pear") => LFS_ERR_NOTEMPTY; for (int i = 0; i < $LARGESIZE; i++) { - sprintf((char*)buffer, "prickly-pear/test%d", i); + sprintf((char*)buffer, "prickly-pear/tedd%d", i); lfs_remove(&lfs, (char*)buffer) => 0; }