Skip to content

Commit 85a9638

Browse files
gekygeky
committed
Fixed issues discovered around testing moves
lfs_dir_fetchwith did not recover from failed dir fetches correctly, added a temporary dir variable to hold dir contents while being populated, allowing us to fall back to a known good dir state if a commit is corrupted. There is a RAM cost, but the upside is that our lfs_dir_fetchwith actually works. Also added better handling of move ids during some get functions.
1 parent 483d41c commit 85a9638

File tree

2 files changed

+41
-26
lines changed

2 files changed

+41
-26
lines changed

‎lfs.c

Lines changed: 38 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -809,10 +809,12 @@ static int lfs_dir_fetchwith(lfs_t *lfs,
809809
lfs_crc(&crc, &dir->rev, sizeof(dir->rev));
810810
dir->rev = lfs_fromle32(dir->rev);
811811

812+
lfs_dir_t temp = *dir;
813+
812814
while (true) {
813815
// extract next tag
814816
lfs_tag_t tag;
815-
int err = lfs_bd_read(lfs, dir->pair[0], off, &tag, sizeof(tag));
817+
int err = lfs_bd_read(lfs, temp.pair[0], off, &tag, sizeof(tag));
816818
if (err) {
817819
return err;
818820
}
@@ -831,18 +833,18 @@ static int lfs_dir_fetchwith(lfs_t *lfs,
831833
break;
832834
}
833835

834-
//printf("tag r %#010x (%x:%x %03x %03x %03x)\n", tag, dir->pair[0], off+sizeof(tag), lfs_tag_type(tag), lfs_tag_id(tag), lfs_tag_size(tag));
836+
//printf("tag r %#010x (%x:%x %03x %03x %03x)\n", tag, temp.pair[0], off+sizeof(tag), lfs_tag_type(tag), lfs_tag_id(tag), lfs_tag_size(tag));
835837
if (lfs_tag_type(tag) == LFS_TYPE_CRC) {
836838
// check the crc entry
837839
uint32_t dcrc;
838-
int err = lfs_bd_read(lfs, dir->pair[0],
840+
int err = lfs_bd_read(lfs, temp.pair[0],
839841
off+sizeof(tag), &dcrc, sizeof(dcrc));
840842
if (err) {
841843
return err;
842844
}
843845

844846
if (crc != lfs_fromle32(dcrc)) {
845-
if (off == sizeof(dir->rev)) {
847+
if (off == sizeof(temp.rev)) {
846848
// try other block
847849
break;
848850
} else {
@@ -852,49 +854,50 @@ static int lfs_dir_fetchwith(lfs_t *lfs,
852854
}
853855
}
854856

855-
dir->off = off + sizeof(tag)+lfs_tag_size(tag);
856-
dir->etag = tag;
857+
temp.off = off + sizeof(tag)+lfs_tag_size(tag);
858+
temp.etag = tag;
857859
crc = 0xffffffff;
860+
*dir = temp;
858861
} else {
859-
err = lfs_bd_crc(lfs, dir->pair[0],
862+
err = lfs_bd_crc(lfs, temp.pair[0],
860863
off+sizeof(tag), lfs_tag_size(tag), &crc);
861864
if (err) {
862865
return err;
863866
}
864867

865868
if (lfs_tag_type(tag) == LFS_TYPE_SOFTTAIL ||
866869
lfs_tag_type(tag) == LFS_TYPE_HARDTAIL) {
867-
dir->split = lfs_tag_type(tag) == LFS_TYPE_HARDTAIL;
868-
err = lfs_bd_read(lfs, dir->pair[0], off+sizeof(tag),
869-
dir->tail, sizeof(dir->tail));
870+
temp.split = lfs_tag_type(tag) == LFS_TYPE_HARDTAIL;
871+
err = lfs_bd_read(lfs, temp.pair[0], off+sizeof(tag),
872+
temp.tail, sizeof(temp.tail));
870873
if (err) {
871874
return err;
872875
}
873876
} else if (lfs_tag_type(tag) == LFS_TYPE_MOVE) {
874877
// TODO handle moves correctly?
875-
dir->moveid = lfs_tag_id(tag);
878+
temp.moveid = lfs_tag_id(tag);
876879
} else {
877880
if (lfs_tag_id(tag) < 0x1ff &&
878-
lfs_tag_id(tag) >= dir->count) {
879-
dir->count = lfs_tag_id(tag)+1;
881+
lfs_tag_id(tag) >= temp.count) {
882+
temp.count = lfs_tag_id(tag)+1;
880883
}
881884

882885
if (lfs_tag_type(tag) == LFS_TYPE_DELETE) {
883-
dir->count -= 1;
884-
if (dir->moveid != -1) {
885-
//printf("RENAME DEL %d (%d)\n", lfs_tag_id(tag), dir->moveid);
886+
temp.count -= 1;
887+
if (temp.moveid != -1) {
888+
//printf("RENAME DEL %d (%d)\n", lfs_tag_id(tag), temp.moveid);
886889
}
887-
if (lfs_tag_id(tag) == dir->moveid) {
888-
dir->moveid = -1;
889-
} else if (lfs_tag_id(tag) < dir->moveid) {
890-
dir->moveid -= 1;
890+
if (lfs_tag_id(tag) == temp.moveid) {
891+
temp.moveid = -1;
892+
} else if (lfs_tag_id(tag) < temp.moveid) {
893+
temp.moveid -= 1;
891894
}
892895
}
893896

894897
if (cb) {
895898
err = cb(lfs, data, (lfs_entry_t){
896899
(tag | 0x80000000),
897-
.u.d.block=dir->pair[0],
900+
.u.d.block=temp.pair[0],
898901
.u.d.off=off+sizeof(tag)});
899902
if (err) {
900903
return err;
@@ -1617,6 +1620,17 @@ static int lfs_dir_getinfo(lfs_t *lfs, lfs_dir_t *dir,
16171620
return 0;
16181621
}
16191622

1623+
if (id == dir->moveid) {
1624+
int moved = lfs_moved(lfs, dir, dir->moveid);
1625+
if (moved < 0) {
1626+
return moved;
1627+
}
1628+
1629+
if (moved) {
1630+
return LFS_ERR_NOENT;
1631+
}
1632+
}
1633+
16201634
lfs_entry_t entry;
16211635
int err = lfs_dir_getentry(lfs, dir, 0x701ff000,
16221636
lfs_mktag(LFS_TYPE_REG, id, 0), &entry);
@@ -4680,8 +4694,9 @@ static int lfs_moved(lfs_t *lfs, lfs_dir_t *fromdir, uint16_t fromid) {
46804694
// grab entry pair we're looking for
46814695
fromdir->moveid = -1;
46824696
lfs_entry_t fromentry;
4683-
int err = lfs_dir_getentry(lfs, fromdir, 0x43dff000,
4684-
lfs_mktag(LFS_STRUCT_DIR, fromid, 0), &fromentry);
4697+
// TODO what about inline files?
4698+
int err = lfs_dir_getentry(lfs, fromdir, 0x401ff000,
4699+
lfs_mktag(LFS_TYPE_REG, fromid, 0), &fromentry);
46854700
fromdir->moveid = fromid;
46864701
if (err) {
46874702
return err;

‎tests/test_move.sh

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -59,7 +59,7 @@ tests/test.py << TEST
5959
lfs_rename(&lfs, "b/hello", "c/hello") => 0;
6060
lfs_unmount(&lfs) => 0;
6161
TEST
62-
rm -v blocks/7
62+
truncate -s-7 blocks/6
6363
tests/test.py << TEST
6464
lfs_mount(&lfs, &cfg) => 0;
6565
lfs_dir_open(&lfs, &dir[0], "b") => 0;
@@ -86,8 +86,8 @@ tests/test.py << TEST
8686
lfs_rename(&lfs, "c/hello", "d/hello") => 0;
8787
lfs_unmount(&lfs) => 0;
8888
TEST
89-
rm -v blocks/8
90-
rm -v blocks/a
89+
truncate -s-7 blocks/8
90+
truncate -s-7 blocks/a
9191
tests/test.py << TEST
9292
lfs_mount(&lfs, &cfg) => 0;
9393
lfs_dir_open(&lfs, &dir[0], "c") => 0;

0 commit comments

Comments
 (0)