Skip to content

Commit 43e3009

Browse files
kripkenkripken
authored
[WasmFS] Allow removeChild to return specific error codes, and use it in OPFS (emscripten-core#17793)
1 parent 829b933 commit 43e3009

10 files changed

Lines changed: 64 additions & 37 deletions

File tree

‎src/library_wasmfs_opfs.js‎

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -202,10 +202,15 @@ mergeInto(LibraryManager.library, {
202202

203203
_wasmfs_opfs_remove_child__deps: ['$wasmfsOPFSFree',
204204
'$wasmfsOPFSDirectoryHandles'],
205-
_wasmfs_opfs_remove_child: async function(ctx, dirID, namePtr) {
205+
_wasmfs_opfs_remove_child: async function(ctx, dirID, namePtr, errPtr) {
206206
let name = UTF8ToString(namePtr);
207207
let dirHandle = wasmfsOPFSDirectoryHandles.get(dirID);
208-
await dirHandle.removeEntry(name);
208+
try {
209+
await dirHandle.removeEntry(name);
210+
} catch {
211+
let err = -{{{ cDefine('EIO') }}};
212+
{{{ makeSetValue('errPtr', 0, 'err', 'i32') }}};
213+
}
209214
_emscripten_proxy_finish(ctx);
210215
},
211216

‎system/lib/wasmfs/backends/memory_backend.cpp‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -45,13 +45,13 @@ std::shared_ptr<File> MemoryDirectory::getChild(const std::string& name) {
4545
return nullptr;
4646
}
4747

48-
bool MemoryDirectory::removeChild(const std::string& name) {
48+
int MemoryDirectory::removeChild(const std::string& name) {
4949
auto entry = findEntry(name);
5050
if (entry != entries.end()) {
5151
entry->child->locked().setParent(nullptr);
5252
entries.erase(entry);
5353
}
54-
return true;
54+
return 0;
5555
}
5656

5757
std::vector<Directory::Entry> MemoryDirectory::getEntries() {
@@ -74,7 +74,7 @@ int MemoryDirectory::insertMove(const std::string& name,
7474
break;
7575
}
7676
}
77-
removeChild(name);
77+
(void)removeChild(name);
7878
insertChild(name, file);
7979
return 0;
8080
}

‎system/lib/wasmfs/backends/node_backend.cpp‎

Lines changed: 3 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -217,19 +217,16 @@ class NodeDirectory : public Directory {
217217
}
218218
}
219219

220-
bool removeChild(const std::string& name) override {
220+
int removeChild(const std::string& name) override {
221221
auto childPath = getChildPath(name);
222222
// Try both `unlink` and `rmdir`.
223223
if (auto err = _wasmfs_node_unlink(childPath.c_str())) {
224224
if (err == EISDIR) {
225225
err = _wasmfs_node_rmdir(childPath.c_str());
226226
}
227-
if (err) {
228-
// TODO: Report specific errors.
229-
return false;
230-
}
227+
return -err;
231228
}
232-
return true;
229+
return 0;
233230
}
234231

235232
std::shared_ptr<DataFile> insertDataFile(const std::string& name,

‎system/lib/wasmfs/backends/opfs_backend.cpp‎

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -47,7 +47,8 @@ void _wasmfs_opfs_move(em_proxying_ctx* ctx,
4747

4848
void _wasmfs_opfs_remove_child(em_proxying_ctx* ctx,
4949
int dir_id,
50-
const char* name);
50+
const char* name,
51+
int* err);
5152

5253
void _wasmfs_opfs_get_entries(em_proxying_ctx* ctx,
5354
int dirID,
@@ -411,11 +412,12 @@ class OPFSDirectory : public Directory {
411412
return err;
412413
}
413414

414-
bool removeChild(const std::string& name) override {
415+
int removeChild(const std::string& name) override {
416+
int err = 0;
415417
proxy([&](auto ctx) {
416-
_wasmfs_opfs_remove_child(ctx.ctx, dirID, name.c_str());
418+
_wasmfs_opfs_remove_child(ctx.ctx, dirID, name.c_str(), &err);
417419
});
418-
return true;
420+
return err;
419421
}
420422

421423
size_t getNumEntries() override { return getEntries().size(); }

‎system/lib/wasmfs/file.cpp‎

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -192,23 +192,24 @@ int Directory::Handle::insertMove(const std::string& name,
192192
return 0;
193193
}
194194

195-
bool Directory::Handle::removeChild(const std::string& name) {
195+
int Directory::Handle::removeChild(const std::string& name) {
196196
auto& dcache = getDir()->dcache;
197197
auto entry = dcache.find(name);
198198
// If this is a mount, we don't need to call into the backend.
199199
if (entry != dcache.end() && entry->second.kind == DCacheKind::Mount) {
200200
dcache.erase(entry);
201-
return true;
201+
return 0;
202202
}
203-
if (!getDir()->removeChild(name)) {
204-
return false;
203+
if (auto err = getDir()->removeChild(name)) {
204+
assert(err < 0);
205+
return err;
205206
}
206207
if (entry != dcache.end()) {
207208
entry->second.file->locked().setParent(nullptr);
208209
dcache.erase(entry);
209210
}
210211
setMTime(time(NULL));
211-
return true;
212+
return 0;
212213
}
213214

214215
std::string Directory::Handle::getName(std::shared_ptr<File> file) {

‎system/lib/wasmfs/file.h‎

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -208,10 +208,10 @@ class Directory : public File {
208208
virtual int insertMove(const std::string& name,
209209
std::shared_ptr<File> file) = 0;
210210

211-
// Remove the file with the given name, returning `true` on success or if the
212-
// child has already been removed or returning `false` if the child cannot be
213-
// removed.
214-
virtual bool removeChild(const std::string& name) = 0;
211+
// Remove the file with the given name. Returns zero on success or if the
212+
// child has already been removed and otherwise returns a negative error code
213+
// if the child cannot be removed.
214+
virtual int removeChild(const std::string& name) = 0;
215215

216216
// The number of entries in this directory.
217217
virtual size_t getNumEntries() = 0;
@@ -377,10 +377,10 @@ class Directory::Handle : public File::Handle {
377377
[[nodiscard]] int insertMove(const std::string& name,
378378
std::shared_ptr<File> file);
379379

380-
// Remove the file with the given name, returning `true` on success or if the
381-
// vhild has already been removed or returning `false` if the child cannot be
382-
// removed.
383-
bool removeChild(const std::string& name);
380+
// Remove the file with the given name. Returns zero on success or if the
381+
// child has already been removed and otherwise returns a negative error code
382+
// if the child cannot be removed.
383+
[[nodiscard]] int removeChild(const std::string& name);
384384

385385
std::string getName(std::shared_ptr<File> file);
386386

‎system/lib/wasmfs/memory_backend.h‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -63,7 +63,7 @@ class MemoryDirectory : public Directory {
6363

6464
std::shared_ptr<File> getChild(const std::string& name) override;
6565

66-
bool removeChild(const std::string& name) override;
66+
int removeChild(const std::string& name) override;
6767

6868
std::shared_ptr<DataFile> insertDataFile(const std::string& name,
6969
mode_t mode) override {

‎system/lib/wasmfs/syscalls.cpp‎

Lines changed: 2 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -834,10 +834,7 @@ int __syscall_unlinkat(int dirfd, intptr_t path, int flags) {
834834
}
835835

836836
// Input is valid, perform the unlink.
837-
if (!lockedParent.removeChild(childName)) {
838-
return -EPERM;
839-
}
840-
return 0;
837+
return lockedParent.removeChild(childName);
841838
}
842839

843840
int __syscall_rmdir(intptr_t path) {
@@ -1014,10 +1011,7 @@ int __syscall_renameat(int olddirfd,
10141011
}
10151012

10161013
// Perform the move.
1017-
if (auto err = lockedNewParent.insertMove(newFileName, oldFile)) {
1018-
return err;
1019-
}
1020-
return 0;
1014+
return lockedNewParent.insertMove(newFileName, oldFile);
10211015
}
10221016

10231017
int __syscall_rename(intptr_t oldpath, intptr_t newpath) {

‎test/wasmfs/wasmfs_opfs_errors.c‎

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -64,6 +64,19 @@ int try_truncate(void) {
6464
return 2;
6565
}
6666

67+
EMSCRIPTEN_KEEPALIVE
68+
int try_unlink(void) {
69+
int err = unlink(file);
70+
if (err == 0) {
71+
return 1;
72+
}
73+
if (errno == EIO) {
74+
return 0;
75+
}
76+
emscripten_console_error(strerror(errno));
77+
return 2;
78+
}
79+
6780
EMSCRIPTEN_KEEPALIVE
6881
int try_oob_read(void) {
6982
int fd = open(file, O_RDWR);
@@ -74,12 +87,15 @@ int try_oob_read(void) {
7487
char buf;
7588
int nread = pread(fd, &buf, 1, (off_t)-1ll);
7689
if (nread > 0) {
90+
close(fd);
7791
return 1;
7892
}
7993
if (errno == EINVAL) {
94+
close(fd);
8095
return 0;
8196
}
8297
EM_ASM({ console.log('errno', $0); }, errno);
98+
close(fd);
8399
return 2;
84100
}
85101

@@ -93,12 +109,15 @@ int try_oob_write(void) {
93109
char buf = 0;
94110
int nread = pwrite(fd, &buf, 1, (off_t)-1ll);
95111
if (nread > 0) {
112+
close(fd);
96113
return 1;
97114
}
98115
if (errno == EINVAL) {
116+
close(fd);
99117
return 0;
100118
}
101119
emscripten_console_error(strerror(errno));
120+
close(fd);
102121
return 2;
103122
}
104123

‎test/wasmfs/wasmfs_opfs_errors_post.js‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,10 @@ async function run_test() {
2828
throw "Did not get expected EIO when resizing file";
2929
}
3030

31+
if (Module._try_unlink() != 0) {
32+
throw "Did not get expected EIO when unlinking file";
33+
}
34+
3135
await access.close();
3236

3337
// We can open the file in any mode now that there is no open access
@@ -57,5 +61,10 @@ async function run_test() {
5761
throw "Did not get expected EINVAL doing out of bounds write";
5862
}
5963

64+
if (Module._try_unlink() != 1) {
65+
throw "Did not succeed to unlink the file (which should work now that " +
66+
"nothing prevents it)";
67+
}
68+
6069
Module._report_result(0);
6170
}

0 commit comments

Comments
 (0)