Skip to content

Commit

Permalink
Added directory list for synchronizing in flight directories
Browse files Browse the repository at this point in the history
As it was, if a user operated on a directory while at the same
time iterating over the directory, the directory objects could
fall out of sync. In the best case, files may be skipped while
removing everything in a file, in the worst case, a very poorly
timed directory relocate could be missed.

Simple fix is to add the same directory tracking that is currently
in use for files, at a small code+complexity cost.
  • Loading branch information
geky committed Nov 22, 2017
1 parent e169d06 commit bf78b09
Show file tree
Hide file tree
Showing 3 changed files with 100 additions and 28 deletions.
80 changes: 55 additions & 25 deletions lfs.c
Original file line number Diff line number Diff line change
Expand Up @@ -569,7 +569,18 @@ static int lfs_dir_commit(lfs_t *lfs, lfs_dir_t *dir,
// update references if we relocated
LFS_DEBUG("Relocating %d %d to %d %d",
oldpair[0], oldpair[1], dir->pair[0], dir->pair[1]);
return lfs_relocate(lfs, oldpair, dir->pair);
int err = lfs_relocate(lfs, oldpair, dir->pair);
if (err) {
return err;
}
}

// shift over any directories that are affected
for (lfs_dir_t *d = lfs->dirs; d; d = d->next) {
if (lfs_paircmp(d->pair, dir->pair) == 0) {
d->pair[0] = dir->pair[0];
d->pair[1] = dir->pair[1];
}
}

return 0;
Expand Down Expand Up @@ -628,7 +639,7 @@ static int lfs_dir_append(lfs_t *lfs, lfs_dir_t *dir,
}

static int lfs_dir_remove(lfs_t *lfs, lfs_dir_t *dir, lfs_entry_t *entry) {
// either shift out the one entry or remove the whole dir block
// check if we should just drop the directory block
if ((dir->d.size & 0x7fffffff) == sizeof(dir->d)+4
+ lfs_entry_size(entry)) {
lfs_dir_t pdir;
Expand All @@ -637,38 +648,44 @@ static int lfs_dir_remove(lfs_t *lfs, lfs_dir_t *dir, lfs_entry_t *entry) {
return res;
}

if (!(pdir.d.size & 0x80000000)) {
return lfs_dir_commit(lfs, dir, (struct lfs_region[]){
{entry->off, lfs_entry_size(entry), NULL, 0},
}, 1);
} else {
if (pdir.d.size & 0x80000000) {
pdir.d.size &= dir->d.size | 0x7fffffff;
pdir.d.tail[0] = dir->d.tail[0];
pdir.d.tail[1] = dir->d.tail[1];
return lfs_dir_commit(lfs, &pdir, NULL, 0);
}
} else {
int err = lfs_dir_commit(lfs, dir, (struct lfs_region[]){
{entry->off, lfs_entry_size(entry), NULL, 0},
}, 1);
if (err) {
return err;
}
}

// shift over any files that are affected
for (lfs_file_t *f = lfs->files; f; f = f->next) {
if (lfs_paircmp(f->pair, dir->pair) == 0) {
if (f->poff == entry->off) {
f->pair[0] = 0xffffffff;
f->pair[1] = 0xffffffff;
} else if (f->poff > entry->off) {
f->poff -= lfs_entry_size(entry);
}
// shift out the entry
int err = lfs_dir_commit(lfs, dir, (struct lfs_region[]){
{entry->off, lfs_entry_size(entry), NULL, 0},
}, 1);
if (err) {
return err;
}

// shift over any files/directories that are affected
for (lfs_file_t *f = lfs->files; f; f = f->next) {
if (lfs_paircmp(f->pair, dir->pair) == 0) {
if (f->poff == entry->off) {
f->pair[0] = 0xffffffff;
f->pair[1] = 0xffffffff;
} else if (f->poff > entry->off) {
f->poff -= lfs_entry_size(entry);
}
}
}

return 0;
for (lfs_dir_t *d = lfs->dirs; d; d = d->next) {
if (lfs_paircmp(d->pair, dir->pair) == 0) {
if (d->off > entry->off) {
d->off -= lfs_entry_size(entry);
d->pos -= lfs_entry_size(entry);
}
}
}

return 0;
}

static int lfs_dir_next(lfs_t *lfs, lfs_dir_t *dir, lfs_entry_t *entry) {
Expand Down Expand Up @@ -894,11 +911,23 @@ int lfs_dir_open(lfs_t *lfs, lfs_dir_t *dir, const char *path) {
dir->head[1] = dir->pair[1];
dir->pos = sizeof(dir->d) - 2;
dir->off = sizeof(dir->d);

// add to list of directories
dir->next = lfs->dirs;
lfs->dirs = dir;

return 0;
}

int lfs_dir_close(lfs_t *lfs, lfs_dir_t *dir) {
// do nothing, dir is always synchronized
// remove from list of directories
for (lfs_dir_t **p = &lfs->dirs; *p; p = &(*p)->next) {
if (*p == dir) {
*p = dir->next;
break;
}
}

return 0;
}

Expand Down Expand Up @@ -1902,6 +1931,7 @@ static int lfs_init(lfs_t *lfs, const struct lfs_config *cfg) {
lfs->root[0] = 0xffffffff;
lfs->root[1] = 0xffffffff;
lfs->files = NULL;
lfs->dirs = NULL;
lfs->deorphaned = false;

return 0;
Expand Down
2 changes: 2 additions & 0 deletions lfs.h
Original file line number Diff line number Diff line change
Expand Up @@ -207,6 +207,7 @@ typedef struct lfs_file {
} lfs_file_t;

typedef struct lfs_dir {
struct lfs_dir *next;
lfs_block_t pair[2];
lfs_off_t off;

Expand Down Expand Up @@ -249,6 +250,7 @@ typedef struct lfs {

lfs_block_t root[2];
lfs_file_t *files;
lfs_dir_t *dirs;

lfs_cache_t rcache;
lfs_cache_t pcache;
Expand Down
46 changes: 43 additions & 3 deletions tests/test_dirs.sh
Original file line number Diff line number Diff line change
Expand Up @@ -282,6 +282,49 @@ tests/test.py << TEST
lfs_unmount(&lfs) => 0;
TEST

echo "--- Recursive remove ---"
tests/test.py << TEST
lfs_mount(&lfs, &cfg) => 0;
lfs_remove(&lfs, "coldpotato") => LFS_ERR_INVAL;
lfs_dir_open(&lfs, &dir[0], "coldpotato") => 0;
lfs_dir_read(&lfs, &dir[0], &info) => 1;
lfs_dir_read(&lfs, &dir[0], &info) => 1;
while (true) {
int err = lfs_dir_read(&lfs, &dir[0], &info);
err >= 0 => 1;
if (err == 0) {
break;
}
strcpy((char*)buffer, "coldpotato/");
strcat((char*)buffer, info.name);
lfs_remove(&lfs, (char*)buffer) => 0;
}
lfs_remove(&lfs, "coldpotato") => 0;
TEST
tests/test.py << TEST
lfs_mount(&lfs, &cfg) => 0;
lfs_dir_open(&lfs, &dir[0], "/") => 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;
lfs_dir_read(&lfs, &dir[0], &info) => 1;
strcmp(info.name, "burito") => 0;
info.type => LFS_TYPE_REG;
lfs_dir_read(&lfs, &dir[0], &info) => 1;
strcmp(info.name, "cactus") => 0;
info.type => LFS_TYPE_DIR;
lfs_dir_read(&lfs, &dir[0], &info) => 0;
lfs_dir_close(&lfs, &dir[0]) => 0;
lfs_unmount(&lfs) => 0;
TEST

echo "--- Multi-block remove ---"
tests/test.py << TEST
lfs_mount(&lfs, &cfg) => 0;
Expand All @@ -307,9 +350,6 @@ tests/test.py << TEST
lfs_dir_read(&lfs, &dir[0], &info) => 1;
strcmp(info.name, "burito") => 0;
info.type => LFS_TYPE_REG;
lfs_dir_read(&lfs, &dir[0], &info) => 1;
strcmp(info.name, "coldpotato") => 0;
info.type => LFS_TYPE_DIR;
lfs_dir_read(&lfs, &dir[0], &info) => 0;
lfs_dir_close(&lfs, &dir[0]) => 0;
lfs_unmount(&lfs) => 0;
Expand Down

0 comments on commit bf78b09

Please sign in to comment.