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
This commit is contained in:
Christopher Haster
2018-10-07 13:47:29 -05:00
parent 0bb1f7af17
commit 97d8d5e96a
3 changed files with 80 additions and 23 deletions

39
lfs.c
View File

@@ -888,7 +888,7 @@ nextname:
}
// 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;