Skip to content

Commit 3ce1f72

Browse files
authored
[WasmFS] Allow move to return specific error codes (#17785)
It already supported returning errors via a `bool`, but it is better to be able to return precise errors. Also catch and return errors in the OPFS backend, which did not previously do any error handling for moves. Tested manually via an error injected into the OPFS backend since I don't know a way to get the `move` method on the FileSystemFileHandle to reliably fail in a way that wouldn't cause the syscall implementation to bail out earlier.
1 parent 44fd8b1 commit 3ce1f72

File tree

8 files changed

+35
-31
lines changed

8 files changed

+35
-31
lines changed

src/library_wasmfs_opfs.js

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -187,12 +187,16 @@ mergeInto(LibraryManager.library, {
187187

188188
_wasmfs_opfs_move__deps: ['$wasmfsOPFSFileHandles',
189189
'$wasmfsOPFSDirectoryHandles'],
190-
_wasmfs_opfs_move: async function(ctx, fileID, newDirID, namePtr) {
190+
_wasmfs_opfs_move: async function(ctx, fileID, newDirID, namePtr, errPtr) {
191191
let name = UTF8ToString(namePtr);
192192
let fileHandle = wasmfsOPFSFileHandles.get(fileID);
193193
let newDirHandle = wasmfsOPFSDirectoryHandles.get(newDirID);
194-
// TODO: error handling
195-
await fileHandle.move(newDirHandle, name);
194+
try {
195+
await fileHandle.move(newDirHandle, name);
196+
} catch {
197+
let err = -{{{ cDefine('EIO') }}};
198+
{{{ makeSetValue('errPtr', 0, 'err', 'i32') }}};
199+
}
196200
_emscripten_proxy_finish(ctx);
197201
},
198202

system/lib/wasmfs/backends/memory_backend.cpp

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -63,8 +63,8 @@ std::vector<Directory::Entry> MemoryDirectory::getEntries() {
6363
return result;
6464
}
6565

66-
bool MemoryDirectory::insertMove(const std::string& name,
67-
std::shared_ptr<File> file) {
66+
int MemoryDirectory::insertMove(const std::string& name,
67+
std::shared_ptr<File> file) {
6868
auto& oldEntries =
6969
std::static_pointer_cast<MemoryDirectory>(file->locked().getParent())
7070
->entries;
@@ -76,7 +76,7 @@ bool MemoryDirectory::insertMove(const std::string& name,
7676
}
7777
removeChild(name);
7878
insertChild(name, file);
79-
return true;
79+
return 0;
8080
}
8181

8282
std::string MemoryDirectory::getName(std::shared_ptr<File> file) {

system/lib/wasmfs/backends/node_backend.cpp

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -256,8 +256,7 @@ class NodeDirectory : public Directory {
256256
abort();
257257
}
258258

259-
bool insertMove(const std::string& name,
260-
std::shared_ptr<File> file) override {
259+
int insertMove(const std::string& name, std::shared_ptr<File> file) override {
261260
// TODO
262261
abort();
263262
}

system/lib/wasmfs/backends/opfs_backend.cpp

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,8 @@ void _wasmfs_opfs_insert_directory(em_proxying_ctx* ctx,
4242
void _wasmfs_opfs_move(em_proxying_ctx* ctx,
4343
int file_id,
4444
int new_dir_id,
45-
const char* name);
45+
const char* name,
46+
int* err);
4647

4748
void _wasmfs_opfs_remove_child(em_proxying_ctx* ctx,
4849
int dir_id,
@@ -401,14 +402,13 @@ class OPFSDirectory : public Directory {
401402
return nullptr;
402403
}
403404

404-
bool insertMove(const std::string& name,
405-
std::shared_ptr<File> file) override {
405+
int insertMove(const std::string& name, std::shared_ptr<File> file) override {
406406
auto old_file = std::static_pointer_cast<OPFSFile>(file);
407+
int err = 0;
407408
proxy([&](auto ctx) {
408-
_wasmfs_opfs_move(ctx.ctx, old_file->fileID, dirID, name.c_str());
409+
_wasmfs_opfs_move(ctx.ctx, old_file->fileID, dirID, name.c_str(), &err);
409410
});
410-
// TODO: Handle errors.
411-
return true;
411+
return err;
412412
}
413413

414414
bool removeChild(const std::string& name) override {

system/lib/wasmfs/file.cpp

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -143,11 +143,11 @@ Directory::Handle::insertSymlink(const std::string& name,
143143
// TODO: consider moving this to be `Backend::move` to avoid asymmetry between
144144
// the source and destination directories and/or taking `Directory::Handle`
145145
// arguments to prove that the directories have already been locked.
146-
bool Directory::Handle::insertMove(const std::string& name,
147-
std::shared_ptr<File> file) {
146+
int Directory::Handle::insertMove(const std::string& name,
147+
std::shared_ptr<File> file) {
148148
// Cannot insert into an unlinked directory.
149149
if (!getParent()) {
150-
return false;
150+
return -EPERM;
151151
}
152152

153153
// Look up the file in its old parent's cache.
@@ -161,8 +161,8 @@ bool Directory::Handle::insertMove(const std::string& name,
161161
// involving the backend.
162162

163163
// Attempt the move.
164-
if (!getDir()->insertMove(name, file)) {
165-
return false;
164+
if (auto err = getDir()->insertMove(name, file)) {
165+
return err;
166166
}
167167

168168
if (oldIt != oldCache.end()) {
@@ -189,7 +189,7 @@ bool Directory::Handle::insertMove(const std::string& name,
189189
oldParent->locked().setMTime(now);
190190
setMTime(now);
191191

192-
return true;
192+
return 0;
193193
}
194194

195195
bool Directory::Handle::removeChild(const std::string& name) {

system/lib/wasmfs/file.h

Lines changed: 9 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -203,10 +203,10 @@ class Directory : public File {
203203
// Move the file represented by `file` from its current directory to this
204204
// directory with the new `name`, possibly overwriting another file that
205205
// already exists with that name. The old directory may be the same as this
206-
// directory. On success, return `true`. Otherwise return `false` without
207-
// changing any underlying state.
208-
virtual bool insertMove(const std::string& name,
209-
std::shared_ptr<File> file) = 0;
206+
// directory. On success return 0 and otherwise return a negative error code
207+
// without changing any underlying state.
208+
virtual int insertMove(const std::string& name,
209+
std::shared_ptr<File> file) = 0;
210210

211211
// Remove the file with the given name, returning `true` on success or if the
212212
// child has already been removed or returning `false` if the child cannot be
@@ -371,10 +371,11 @@ class Directory::Handle : public File::Handle {
371371
// Move the file represented by `file` from its current directory to this
372372
// directory with the new `name`, possibly overwriting another file that
373373
// already exists with that name. The old directory may be the same as this
374-
// directory. On success, return `true`. Otherwise return `false` without
375-
// changing any underlying state. This should only be called from renameat
376-
// with the locks on the old and new parents already held.
377-
bool insertMove(const std::string& name, std::shared_ptr<File> file);
374+
// directory. On success return 0 and otherwise return a negative error code
375+
// without changing any underlying state. This should only be called from
376+
// renameat with the locks on the old and new parents already held.
377+
[[nodiscard]] int insertMove(const std::string& name,
378+
std::shared_ptr<File> file);
378379

379380
// Remove the file with the given name, returning `true` on success or if the
380381
// vhild has already been removed or returning `false` if the child cannot be

system/lib/wasmfs/memory_backend.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -86,7 +86,7 @@ class MemoryDirectory : public Directory {
8686
return child;
8787
}
8888

89-
bool insertMove(const std::string& name, std::shared_ptr<File> file) override;
89+
int insertMove(const std::string& name, std::shared_ptr<File> file) override;
9090

9191
size_t getNumEntries() override { return entries.size(); }
9292
std::vector<Directory::Entry> getEntries() override;

system/lib/wasmfs/syscalls.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1014,8 +1014,8 @@ int __syscall_renameat(int olddirfd,
10141014
}
10151015

10161016
// Perform the move.
1017-
if (!lockedNewParent.insertMove(newFileName, oldFile)) {
1018-
return -EPERM;
1017+
if (auto err = lockedNewParent.insertMove(newFileName, oldFile)) {
1018+
return err;
10191019
}
10201020
return 0;
10211021
}

0 commit comments

Comments
 (0)