Skip to content

Commit bf78b09

Browse files
committed
Added directory list for synchronizing in flight directories
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.
1 parent e169d06 commit bf78b09

File tree

3 files changed

+100
-28
lines changed

3 files changed

+100
-28
lines changed

lfs.c

Lines changed: 55 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -569,7 +569,18 @@ static int lfs_dir_commit(lfs_t *lfs, lfs_dir_t *dir,
569569
// update references if we relocated
570570
LFS_DEBUG("Relocating %d %d to %d %d",
571571
oldpair[0], oldpair[1], dir->pair[0], dir->pair[1]);
572-
return lfs_relocate(lfs, oldpair, dir->pair);
572+
int err = lfs_relocate(lfs, oldpair, dir->pair);
573+
if (err) {
574+
return err;
575+
}
576+
}
577+
578+
// shift over any directories that are affected
579+
for (lfs_dir_t *d = lfs->dirs; d; d = d->next) {
580+
if (lfs_paircmp(d->pair, dir->pair) == 0) {
581+
d->pair[0] = dir->pair[0];
582+
d->pair[1] = dir->pair[1];
583+
}
573584
}
574585

575586
return 0;
@@ -628,7 +639,7 @@ static int lfs_dir_append(lfs_t *lfs, lfs_dir_t *dir,
628639
}
629640

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

640-
if (!(pdir.d.size & 0x80000000)) {
641-
return lfs_dir_commit(lfs, dir, (struct lfs_region[]){
642-
{entry->off, lfs_entry_size(entry), NULL, 0},
643-
}, 1);
644-
} else {
651+
if (pdir.d.size & 0x80000000) {
645652
pdir.d.size &= dir->d.size | 0x7fffffff;
646653
pdir.d.tail[0] = dir->d.tail[0];
647654
pdir.d.tail[1] = dir->d.tail[1];
648655
return lfs_dir_commit(lfs, &pdir, NULL, 0);
649656
}
650-
} else {
651-
int err = lfs_dir_commit(lfs, dir, (struct lfs_region[]){
652-
{entry->off, lfs_entry_size(entry), NULL, 0},
653-
}, 1);
654-
if (err) {
655-
return err;
656-
}
657+
}
657658

658-
// shift over any files that are affected
659-
for (lfs_file_t *f = lfs->files; f; f = f->next) {
660-
if (lfs_paircmp(f->pair, dir->pair) == 0) {
661-
if (f->poff == entry->off) {
662-
f->pair[0] = 0xffffffff;
663-
f->pair[1] = 0xffffffff;
664-
} else if (f->poff > entry->off) {
665-
f->poff -= lfs_entry_size(entry);
666-
}
659+
// shift out the entry
660+
int err = lfs_dir_commit(lfs, dir, (struct lfs_region[]){
661+
{entry->off, lfs_entry_size(entry), NULL, 0},
662+
}, 1);
663+
if (err) {
664+
return err;
665+
}
666+
667+
// shift over any files/directories that are affected
668+
for (lfs_file_t *f = lfs->files; f; f = f->next) {
669+
if (lfs_paircmp(f->pair, dir->pair) == 0) {
670+
if (f->poff == entry->off) {
671+
f->pair[0] = 0xffffffff;
672+
f->pair[1] = 0xffffffff;
673+
} else if (f->poff > entry->off) {
674+
f->poff -= lfs_entry_size(entry);
667675
}
668676
}
677+
}
669678

670-
return 0;
679+
for (lfs_dir_t *d = lfs->dirs; d; d = d->next) {
680+
if (lfs_paircmp(d->pair, dir->pair) == 0) {
681+
if (d->off > entry->off) {
682+
d->off -= lfs_entry_size(entry);
683+
d->pos -= lfs_entry_size(entry);
684+
}
685+
}
671686
}
687+
688+
return 0;
672689
}
673690

674691
static int lfs_dir_next(lfs_t *lfs, lfs_dir_t *dir, lfs_entry_t *entry) {
@@ -894,11 +911,23 @@ int lfs_dir_open(lfs_t *lfs, lfs_dir_t *dir, const char *path) {
894911
dir->head[1] = dir->pair[1];
895912
dir->pos = sizeof(dir->d) - 2;
896913
dir->off = sizeof(dir->d);
914+
915+
// add to list of directories
916+
dir->next = lfs->dirs;
917+
lfs->dirs = dir;
918+
897919
return 0;
898920
}
899921

900922
int lfs_dir_close(lfs_t *lfs, lfs_dir_t *dir) {
901-
// do nothing, dir is always synchronized
923+
// remove from list of directories
924+
for (lfs_dir_t **p = &lfs->dirs; *p; p = &(*p)->next) {
925+
if (*p == dir) {
926+
*p = dir->next;
927+
break;
928+
}
929+
}
930+
902931
return 0;
903932
}
904933

@@ -1902,6 +1931,7 @@ static int lfs_init(lfs_t *lfs, const struct lfs_config *cfg) {
19021931
lfs->root[0] = 0xffffffff;
19031932
lfs->root[1] = 0xffffffff;
19041933
lfs->files = NULL;
1934+
lfs->dirs = NULL;
19051935
lfs->deorphaned = false;
19061936

19071937
return 0;

lfs.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -207,6 +207,7 @@ typedef struct lfs_file {
207207
} lfs_file_t;
208208

209209
typedef struct lfs_dir {
210+
struct lfs_dir *next;
210211
lfs_block_t pair[2];
211212
lfs_off_t off;
212213

@@ -249,6 +250,7 @@ typedef struct lfs {
249250

250251
lfs_block_t root[2];
251252
lfs_file_t *files;
253+
lfs_dir_t *dirs;
252254

253255
lfs_cache_t rcache;
254256
lfs_cache_t pcache;

tests/test_dirs.sh

Lines changed: 43 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -282,6 +282,49 @@ tests/test.py << TEST
282282
lfs_unmount(&lfs) => 0;
283283
TEST
284284

285+
echo "--- Recursive remove ---"
286+
tests/test.py << TEST
287+
lfs_mount(&lfs, &cfg) => 0;
288+
lfs_remove(&lfs, "coldpotato") => LFS_ERR_INVAL;
289+
290+
lfs_dir_open(&lfs, &dir[0], "coldpotato") => 0;
291+
lfs_dir_read(&lfs, &dir[0], &info) => 1;
292+
lfs_dir_read(&lfs, &dir[0], &info) => 1;
293+
294+
while (true) {
295+
int err = lfs_dir_read(&lfs, &dir[0], &info);
296+
err >= 0 => 1;
297+
if (err == 0) {
298+
break;
299+
}
300+
301+
strcpy((char*)buffer, "coldpotato/");
302+
strcat((char*)buffer, info.name);
303+
lfs_remove(&lfs, (char*)buffer) => 0;
304+
}
305+
306+
lfs_remove(&lfs, "coldpotato") => 0;
307+
TEST
308+
tests/test.py << TEST
309+
lfs_mount(&lfs, &cfg) => 0;
310+
lfs_dir_open(&lfs, &dir[0], "/") => 0;
311+
lfs_dir_read(&lfs, &dir[0], &info) => 1;
312+
strcmp(info.name, ".") => 0;
313+
info.type => LFS_TYPE_DIR;
314+
lfs_dir_read(&lfs, &dir[0], &info) => 1;
315+
strcmp(info.name, "..") => 0;
316+
info.type => LFS_TYPE_DIR;
317+
lfs_dir_read(&lfs, &dir[0], &info) => 1;
318+
strcmp(info.name, "burito") => 0;
319+
info.type => LFS_TYPE_REG;
320+
lfs_dir_read(&lfs, &dir[0], &info) => 1;
321+
strcmp(info.name, "cactus") => 0;
322+
info.type => LFS_TYPE_DIR;
323+
lfs_dir_read(&lfs, &dir[0], &info) => 0;
324+
lfs_dir_close(&lfs, &dir[0]) => 0;
325+
lfs_unmount(&lfs) => 0;
326+
TEST
327+
285328
echo "--- Multi-block remove ---"
286329
tests/test.py << TEST
287330
lfs_mount(&lfs, &cfg) => 0;
@@ -307,9 +350,6 @@ tests/test.py << TEST
307350
lfs_dir_read(&lfs, &dir[0], &info) => 1;
308351
strcmp(info.name, "burito") => 0;
309352
info.type => LFS_TYPE_REG;
310-
lfs_dir_read(&lfs, &dir[0], &info) => 1;
311-
strcmp(info.name, "coldpotato") => 0;
312-
info.type => LFS_TYPE_DIR;
313353
lfs_dir_read(&lfs, &dir[0], &info) => 0;
314354
lfs_dir_close(&lfs, &dir[0]) => 0;
315355
lfs_unmount(&lfs) => 0;

0 commit comments

Comments
 (0)