Skip to content

Commit 63b52c9

Browse files
gekygeky
committed
Added proper handling for removing open files
Conveniently we previously added a linked-list of files for things like this. This should handle most of the corner cases where files are open during strange operations. This also brings up the point that we aren't doing anything similar for directories and don't even have a dir linked-list. After thinking about it for a while, I've decided to leave out this handling for dirs. It will likely be very complicated, with little gains as directories are less used in embedded systems. Additionally, dirs are only open for reading, and corruption will probably just cause the dir iteration to terminate. If needed, correct handling of open directories can be added later.
1 parent 8621b61 commit 63b52c9

File tree

3 files changed

+213
-2
lines changed

3 files changed

+213
-2
lines changed

‎Makefile

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,8 @@ size: $(OBJ)
3131
$(SIZE) -t $^
3232

3333
.SUFFIXES:
34-
test: test_format test_dirs test_files test_seek test_alloc test_paths test_orphan
34+
test: test_format test_dirs test_files test_seek test_parallel \
35+
test_alloc test_paths test_orphan
3536
test_%: tests/test_%.sh
3637
./$<
3738

‎lfs.c

Lines changed: 25 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1155,7 +1155,7 @@ int lfs_file_sync(lfs_t *lfs, lfs_file_t *file) {
11551155
return err;
11561156
}
11571157

1158-
if (file->flags & LFS_F_DIRTY) {
1158+
if ((file->flags & LFS_F_DIRTY) && !lfs_pairisnull(file->pair)) {
11591159
// update dir entry
11601160
lfs_dir_t cwd;
11611161
int err = lfs_dir_fetch(lfs, &cwd, file->pair);
@@ -1406,6 +1406,18 @@ int lfs_remove(lfs_t *lfs, const char *path) {
14061406
return err;
14071407
}
14081408

1409+
// shift over any files that are affected
1410+
for (lfs_file_t *f = lfs->files; f; f = f->next) {
1411+
if (lfs_paircmp(f->pair, cwd.pair) == 0) {
1412+
if (f->poff == entry.off) {
1413+
f->pair[0] = 0xffffffff;
1414+
f->pair[1] = 0xffffffff;
1415+
} else if (f->poff > entry.off) {
1416+
f->poff -= entry.d.len;
1417+
}
1418+
}
1419+
}
1420+
14091421
// if we were a directory, just run a deorphan step, this should
14101422
// collect us, although is expensive
14111423
if (entry.d.type == LFS_TYPE_DIR) {
@@ -1498,6 +1510,18 @@ int lfs_rename(lfs_t *lfs, const char *oldpath, const char *newpath) {
14981510
return err;
14991511
}
15001512

1513+
// shift over any files that are affected
1514+
for (lfs_file_t *f = lfs->files; f; f = f->next) {
1515+
if (lfs_paircmp(f->pair, oldcwd.pair) == 0) {
1516+
if (f->poff == oldentry.off) {
1517+
f->pair[0] = 0xffffffff;
1518+
f->pair[1] = 0xffffffff;
1519+
} else if (f->poff > oldentry.off) {
1520+
f->poff -= oldentry.d.len;
1521+
}
1522+
}
1523+
}
1524+
15011525
// if we were a directory, just run a deorphan step, this should
15021526
// collect us, although is expensive
15031527
if (prevexists && preventry.d.type == LFS_TYPE_DIR) {

‎tests/test_parallel.sh

Lines changed: 186 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,186 @@
1+
#!/bin/bash
2+
set -eu
3+
4+
echo "=== Parallel tests ==="
5+
rm -rf blocks
6+
tests/test.py << TEST
7+
lfs_format(&lfs, &cfg) => 0;
8+
TEST
9+
10+
echo "--- Parallel file test ---"
11+
tests/test.py << TEST
12+
lfs_mount(&lfs, &cfg) => 0;
13+
lfs_file_open(&lfs, &file[0], "a", LFS_O_WRONLY | LFS_O_CREAT) => 0;
14+
lfs_file_open(&lfs, &file[1], "b", LFS_O_WRONLY | LFS_O_CREAT) => 0;
15+
lfs_file_open(&lfs, &file[2], "c", LFS_O_WRONLY | LFS_O_CREAT) => 0;
16+
lfs_file_open(&lfs, &file[3], "d", LFS_O_WRONLY | LFS_O_CREAT) => 0;
17+
18+
for (int i = 0; i < 10; i++) {
19+
lfs_file_write(&lfs, &file[0], (const void*)"a", 1) => 1;
20+
lfs_file_write(&lfs, &file[1], (const void*)"b", 1) => 1;
21+
lfs_file_write(&lfs, &file[2], (const void*)"c", 1) => 1;
22+
lfs_file_write(&lfs, &file[3], (const void*)"d", 1) => 1;
23+
}
24+
25+
lfs_file_close(&lfs, &file[0]);
26+
lfs_file_close(&lfs, &file[1]);
27+
lfs_file_close(&lfs, &file[2]);
28+
lfs_file_close(&lfs, &file[3]);
29+
30+
lfs_dir_open(&lfs, &dir[0], "/") => 0;
31+
lfs_dir_read(&lfs, &dir[0], &info) => 1;
32+
strcmp(info.name, ".") => 0;
33+
info.type => LFS_TYPE_DIR;
34+
lfs_dir_read(&lfs, &dir[0], &info) => 1;
35+
strcmp(info.name, "..") => 0;
36+
info.type => LFS_TYPE_DIR;
37+
lfs_dir_read(&lfs, &dir[0], &info) => 1;
38+
strcmp(info.name, "a") => 0;
39+
info.type => LFS_TYPE_REG;
40+
info.size => 10;
41+
lfs_dir_read(&lfs, &dir[0], &info) => 1;
42+
strcmp(info.name, "b") => 0;
43+
info.type => LFS_TYPE_REG;
44+
info.size => 10;
45+
lfs_dir_read(&lfs, &dir[0], &info) => 1;
46+
strcmp(info.name, "c") => 0;
47+
info.type => LFS_TYPE_REG;
48+
info.size => 10;
49+
lfs_dir_read(&lfs, &dir[0], &info) => 1;
50+
strcmp(info.name, "d") => 0;
51+
info.type => LFS_TYPE_REG;
52+
info.size => 10;
53+
lfs_dir_read(&lfs, &dir[0], &info) => 0;
54+
lfs_dir_close(&lfs, &dir[0]) => 0;
55+
56+
lfs_file_open(&lfs, &file[0], "a", LFS_O_RDONLY) => 0;
57+
lfs_file_open(&lfs, &file[1], "b", LFS_O_RDONLY) => 0;
58+
lfs_file_open(&lfs, &file[2], "c", LFS_O_RDONLY) => 0;
59+
lfs_file_open(&lfs, &file[3], "d", LFS_O_RDONLY) => 0;
60+
61+
for (int i = 0; i < 10; i++) {
62+
lfs_file_read(&lfs, &file[0], buffer, 1) => 1;
63+
buffer[0] => 'a';
64+
lfs_file_read(&lfs, &file[1], buffer, 1) => 1;
65+
buffer[0] => 'b';
66+
lfs_file_read(&lfs, &file[2], buffer, 1) => 1;
67+
buffer[0] => 'c';
68+
lfs_file_read(&lfs, &file[3], buffer, 1) => 1;
69+
buffer[0] => 'd';
70+
}
71+
72+
lfs_file_close(&lfs, &file[0]);
73+
lfs_file_close(&lfs, &file[1]);
74+
lfs_file_close(&lfs, &file[2]);
75+
lfs_file_close(&lfs, &file[3]);
76+
77+
lfs_unmount(&lfs) => 0;
78+
TEST
79+
80+
echo "--- Parallel remove file test ---"
81+
tests/test.py << TEST
82+
lfs_mount(&lfs, &cfg) => 0;
83+
lfs_file_open(&lfs, &file[0], "e", LFS_O_WRONLY | LFS_O_CREAT) => 0;
84+
85+
for (int i = 0; i < 5; i++) {
86+
lfs_file_write(&lfs, &file[0], (const void*)"e", 1) => 1;
87+
}
88+
89+
lfs_remove(&lfs, "a") => 0;
90+
lfs_remove(&lfs, "b") => 0;
91+
lfs_remove(&lfs, "c") => 0;
92+
lfs_remove(&lfs, "d") => 0;
93+
94+
for (int i = 0; i < 5; i++) {
95+
lfs_file_write(&lfs, &file[0], (const void*)"e", 1) => 1;
96+
}
97+
98+
lfs_file_close(&lfs, &file[0]);
99+
100+
lfs_dir_open(&lfs, &dir[0], "/") => 0;
101+
lfs_dir_read(&lfs, &dir[0], &info) => 1;
102+
strcmp(info.name, ".") => 0;
103+
info.type => LFS_TYPE_DIR;
104+
lfs_dir_read(&lfs, &dir[0], &info) => 1;
105+
strcmp(info.name, "..") => 0;
106+
info.type => LFS_TYPE_DIR;
107+
lfs_dir_read(&lfs, &dir[0], &info) => 1;
108+
strcmp(info.name, "e") => 0;
109+
info.type => LFS_TYPE_REG;
110+
info.size => 10;
111+
lfs_dir_read(&lfs, &dir[0], &info) => 0;
112+
lfs_dir_close(&lfs, &dir[0]) => 0;
113+
114+
lfs_file_open(&lfs, &file[0], "e", LFS_O_RDONLY) => 0;
115+
116+
for (int i = 0; i < 10; i++) {
117+
lfs_file_read(&lfs, &file[0], buffer, 1) => 1;
118+
buffer[0] => 'e';
119+
}
120+
121+
lfs_file_close(&lfs, &file[0]);
122+
123+
lfs_unmount(&lfs) => 0;
124+
TEST
125+
126+
echo "--- Remove inconveniently test ---"
127+
tests/test.py << TEST
128+
lfs_mount(&lfs, &cfg) => 0;
129+
lfs_file_open(&lfs, &file[0], "e", LFS_O_WRONLY | LFS_O_TRUNC) => 0;
130+
lfs_file_open(&lfs, &file[1], "f", LFS_O_WRONLY | LFS_O_CREAT) => 0;
131+
lfs_file_open(&lfs, &file[2], "g", LFS_O_WRONLY | LFS_O_CREAT) => 0;
132+
133+
for (int i = 0; i < 5; i++) {
134+
lfs_file_write(&lfs, &file[0], (const void*)"e", 1) => 1;
135+
lfs_file_write(&lfs, &file[1], (const void*)"f", 1) => 1;
136+
lfs_file_write(&lfs, &file[2], (const void*)"g", 1) => 1;
137+
}
138+
139+
lfs_remove(&lfs, "f") => 0;
140+
141+
for (int i = 0; i < 5; i++) {
142+
lfs_file_write(&lfs, &file[0], (const void*)"e", 1) => 1;
143+
lfs_file_write(&lfs, &file[1], (const void*)"f", 1) => 1;
144+
lfs_file_write(&lfs, &file[2], (const void*)"g", 1) => 1;
145+
}
146+
147+
lfs_file_close(&lfs, &file[0]);
148+
lfs_file_close(&lfs, &file[1]);
149+
lfs_file_close(&lfs, &file[2]);
150+
151+
lfs_dir_open(&lfs, &dir[0], "/") => 0;
152+
lfs_dir_read(&lfs, &dir[0], &info) => 1;
153+
strcmp(info.name, ".") => 0;
154+
info.type => LFS_TYPE_DIR;
155+
lfs_dir_read(&lfs, &dir[0], &info) => 1;
156+
strcmp(info.name, "..") => 0;
157+
info.type => LFS_TYPE_DIR;
158+
lfs_dir_read(&lfs, &dir[0], &info) => 1;
159+
strcmp(info.name, "e") => 0;
160+
info.type => LFS_TYPE_REG;
161+
info.size => 10;
162+
lfs_dir_read(&lfs, &dir[0], &info) => 1;
163+
strcmp(info.name, "g") => 0;
164+
info.type => LFS_TYPE_REG;
165+
info.size => 10;
166+
lfs_dir_read(&lfs, &dir[0], &info) => 0;
167+
lfs_dir_close(&lfs, &dir[0]) => 0;
168+
169+
lfs_file_open(&lfs, &file[0], "e", LFS_O_RDONLY) => 0;
170+
lfs_file_open(&lfs, &file[1], "g", LFS_O_RDONLY) => 0;
171+
172+
for (int i = 0; i < 10; i++) {
173+
lfs_file_read(&lfs, &file[0], buffer, 1) => 1;
174+
buffer[0] => 'e';
175+
lfs_file_read(&lfs, &file[1], buffer, 1) => 1;
176+
buffer[0] => 'g';
177+
}
178+
179+
lfs_file_close(&lfs, &file[0]);
180+
lfs_file_close(&lfs, &file[1]);
181+
182+
lfs_unmount(&lfs) => 0;
183+
TEST
184+
185+
echo "--- Results ---"
186+
tests/stats.py

0 commit comments

Comments
 (0)