Skip to content

Commit 4c466c6

Browse files
tlivelytlively
committed
[WasmFS] Report errors from getSize
Update the return type of `getSize` from `size_t` to `off_t`, which allows for negative error codes. `off_t` is more correct anyway, since in 32-bit mode it is possible for a file size to be greater than the max value of `size_t`. Also update the argument to `getSize` from `size_t` to `off_t` to match. I manually verified that this error handling works with an injected error, but I was not able to create a test that could trigger the error naturally.
1 parent c121f74 commit 4c466c6

File tree

11 files changed

+89
-56
lines changed

11 files changed

+89
-56
lines changed

‎src/library_wasmfs_opfs.js‎

Lines changed: 22 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -354,9 +354,13 @@ mergeInto(LibraryManager.library, {
354354
_wasmfs_opfs_get_size_access__deps: ['$wasmfsOPFSAccessHandles'],
355355
_wasmfs_opfs_get_size_access: async function(ctx, accessID, sizePtr) {
356356
let accessHandle = wasmfsOPFSAccessHandles.get(accessID);
357-
// TODO: Error handling
358-
let size = await accessHandle.getSize();
359-
{{{ makeSetValue('sizePtr', 0, 'size', 'i32') }}};
357+
let size;
358+
try {
359+
size = await accessHandle.getSize();
360+
} catch {
361+
size = -{{{ cDefine('EIO') }}};
362+
}
363+
{{{ makeSetValue('sizePtr', 0, 'size', 'i64') }}};
360364
_emscripten_proxy_finish(ctx);
361365
},
362366

@@ -369,14 +373,21 @@ mergeInto(LibraryManager.library, {
369373
_wasmfs_opfs_get_size_file__deps: ['$wasmfsOPFSFileHandles'],
370374
_wasmfs_opfs_get_size_file: async function(ctx, fileID, sizePtr) {
371375
let fileHandle = wasmfsOPFSFileHandles.get(fileID);
372-
// TODO: Error handling
373-
let size = (await fileHandle.getFile()).size;
374-
{{{ makeSetValue('sizePtr', 0, 'size', 'i32') }}};
376+
let size;
377+
try {
378+
size = (await fileHandle.getFile()).size;
379+
} catch {
380+
size = -{{{ cDefine('EIO') }}};
381+
}
382+
{{{ makeSetValue('sizePtr', 0, 'size', 'i64') }}};
375383
_emscripten_proxy_finish(ctx);
376384
},
377385

378386
_wasmfs_opfs_set_size_access__deps: ['$wasmfsOPFSAccessHandles'],
379-
_wasmfs_opfs_set_size_access: async function(ctx, accessID, size, errPtr) {
387+
_wasmfs_opfs_set_size_access: async function(ctx, accessID,
388+
{{{ defineI64Param('size') }}},
389+
errPtr) {
390+
{{{ receiveI64ParamAsDouble('size') }}};
380391
let accessHandle = wasmfsOPFSAccessHandles.get(accessID);
381392
try {
382393
await accessHandle.truncate(size);
@@ -388,7 +399,10 @@ mergeInto(LibraryManager.library, {
388399
},
389400

390401
_wasmfs_opfs_set_size_file__deps: ['$wasmfsOPFSFileHandles'],
391-
_wasmfs_opfs_set_size_file: async function(ctx, fileID, size, errPtr) {
402+
_wasmfs_opfs_set_size_file: async function(ctx, fileID,
403+
{{{ defineI64Param('size') }}},
404+
errPtr) {
405+
{{{ receiveI64ParamAsDouble('size') }}};
392406
let fileHandle = wasmfsOPFSFileHandles.get(fileID);
393407
try {
394408
let writable = await fileHandle.createWritable({keepExistingData: true});

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

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -137,7 +137,7 @@ class NodeFile : public DataFile {
137137
: DataFile(mode, backend), state(path) {}
138138

139139
private:
140-
size_t getSize() override {
140+
off_t getSize() override {
141141
// TODO: This should really be using a 64-bit file size type.
142142
uint32_t size;
143143
if (state.isOpen()) {
@@ -151,10 +151,10 @@ class NodeFile : public DataFile {
151151
return 0;
152152
}
153153
}
154-
return size_t(size);
154+
return off_t(size);
155155
}
156156

157-
int setSize(size_t size) override {
157+
int setSize(off_t size) override {
158158
WASMFS_UNREACHABLE("TODO: implement NodeFile::setSize");
159159
}
160160

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

Lines changed: 11 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -88,23 +88,22 @@ int _wasmfs_opfs_write_access(int access_id,
8888
// Get the size via an AccessHandle.
8989
void _wasmfs_opfs_get_size_access(em_proxying_ctx* ctx,
9090
int access_id,
91-
uint32_t* size);
91+
off_t* size);
9292

93+
// TODO: return 64-byte off_t.
9394
uint32_t _wasmfs_opfs_get_size_blob(int blob_id);
9495

9596
// Get the size of a file handle via a File Blob.
96-
void _wasmfs_opfs_get_size_file(em_proxying_ctx* ctx,
97-
int file_id,
98-
uint32_t* size);
97+
void _wasmfs_opfs_get_size_file(em_proxying_ctx* ctx, int file_id, off_t* size);
9998

10099
void _wasmfs_opfs_set_size_access(em_proxying_ctx* ctx,
101100
int access_id,
102-
uint32_t size,
101+
off_t size,
103102
int* err);
104103

105104
void _wasmfs_opfs_set_size_file(em_proxying_ctx* ctx,
106105
int file_id,
107-
uint32_t size,
106+
off_t size,
108107
int* err);
109108

110109
void _wasmfs_opfs_flush_access(em_proxying_ctx* ctx, int access_id, int* err);
@@ -222,9 +221,8 @@ class OPFSFile : public DataFile {
222221
}
223222

224223
private:
225-
size_t getSize() override {
226-
// TODO: 64-bit sizes.
227-
uint32_t size;
224+
off_t getSize() override {
225+
off_t size;
228226
switch (state.getKind()) {
229227
case OpenState::None:
230228
proxy([&](auto ctx) {
@@ -242,10 +240,10 @@ class OPFSFile : public DataFile {
242240
default:
243241
WASMFS_UNREACHABLE("Unexpected open state");
244242
}
245-
return size_t(size);
243+
return size;
246244
}
247245

248-
int setSize(size_t size) override {
246+
int setSize(off_t size) override {
249247
int err = 0;
250248
switch (state.getKind()) {
251249
case OpenState::Access:
@@ -259,6 +257,8 @@ class OPFSFile : public DataFile {
259257
// become invalidated and refreshing it while ensuring other in-flight
260258
// operations on the same file do not observe the invalidated blob would
261259
// be extremely complicated.
260+
// TODO: Can we assume there are no other in-flight operations on this
261+
// file and do something better here?
262262
return -EIO;
263263
case OpenState::None: {
264264
proxy([&](auto ctx) {

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

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -51,13 +51,13 @@ class ProxiedFile : public DataFile {
5151

5252
// Querying the size of the Proxied File returns the size of the underlying
5353
// file given by the proxying mechanism.
54-
size_t getSize() override {
55-
size_t result;
54+
off_t getSize() override {
55+
off_t result;
5656
proxy([&]() { result = baseFile->locked().getSize(); });
5757
return result;
5858
}
5959

60-
int setSize(size_t size) override {
60+
int setSize(off_t size) override {
6161
WASMFS_UNREACHABLE("TODO: ProxiedFS setSize");
6262
}
6363

‎system/lib/wasmfs/file.h‎

Lines changed: 8 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -101,8 +101,9 @@ class File : public std::enable_shared_from_this<File> {
101101
// A mutex is needed for multiple accesses to the same file.
102102
std::recursive_mutex mutex;
103103

104-
// May be called on files that have not been opened.
105-
virtual size_t getSize() = 0;
104+
// The the size in bytes of a file or return a negative error code. May be
105+
// called on files that have not been opened.
106+
virtual off_t getSize() = 0;
106107

107108
mode_t mode = 0; // User and group mode bits for access permission.
108109

@@ -144,7 +145,7 @@ class DataFile : public File {
144145
// Sets the size of the file to a specific size. If new space is allocated, it
145146
// should be zero-initialized. May be called on files that have not been
146147
// opened. Returns 0 on success or a negative error code.
147-
virtual int setSize(size_t size) = 0;
148+
virtual int setSize(off_t size) = 0;
148149

149150
// Sync the file data to the underlying persistent storage, if any. Returns 0
150151
// on success or a negative error code.
@@ -254,7 +255,7 @@ class Directory : public File {
254255
protected:
255256
// 4096 bytes is the size of a block in ext4.
256257
// This value was also copied from the JS file system.
257-
size_t getSize() override { return 4096; }
258+
off_t getSize() override { return 4096; }
258259
};
259260

260261
class Symlink : public File {
@@ -270,7 +271,7 @@ class Symlink : public File {
270271
virtual std::string getTarget() const = 0;
271272

272273
protected:
273-
size_t getSize() override { return getTarget().size(); }
274+
off_t getSize() override { return getTarget().size(); }
274275
};
275276

276277
class File::Handle {
@@ -286,7 +287,7 @@ class File::Handle {
286287
Handle(std::shared_ptr<File> file) : file(file), lock(file->mutex) {}
287288
Handle(std::shared_ptr<File> file, std::defer_lock_t)
288289
: file(file), lock(file->mutex, std::defer_lock) {}
289-
size_t getSize() { return file->getSize(); }
290+
off_t getSize() { return file->getSize(); }
290291
mode_t getMode() { return file->mode; }
291292
void setMode(mode_t mode) {
292293
// The type bits can never be changed (whether something is a file or a
@@ -325,7 +326,7 @@ class DataFile::Handle : public File::Handle {
325326
return getFile()->write(buf, len, offset);
326327
}
327328

328-
[[nodiscard]] int setSize(size_t size) { return getFile()->setSize(size); }
329+
[[nodiscard]] int setSize(off_t size) { return getFile()->setSize(size); }
329330

330331
// TODO: Design a proper API for flushing files.
331332
[[nodiscard]] int flush() { return getFile()->flush(); }

‎system/lib/wasmfs/js_impl_backend.h‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -96,11 +96,11 @@ class JSImplFile : public DataFile {
9696

9797
int flush() override { return 0; }
9898

99-
size_t getSize() override {
99+
off_t getSize() override {
100100
return _wasmfs_jsimpl_get_size(getBackendIndex(), getFileIndex());
101101
}
102102

103-
int setSize(size_t size) override {
103+
int setSize(off_t size) override {
104104
WASMFS_UNREACHABLE("TODO: JSImpl setSize");
105105
}
106106

‎system/lib/wasmfs/memory_backend.h‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -24,8 +24,8 @@ class MemoryFile : public DataFile {
2424
ssize_t write(const uint8_t* buf, size_t len, off_t offset) override;
2525
ssize_t read(uint8_t* buf, size_t len, off_t offset) override;
2626
int flush() override { return 0; }
27-
size_t getSize() override { return buffer.size(); }
28-
int setSize(size_t size) override {
27+
off_t getSize() override { return buffer.size(); }
28+
int setSize(off_t size) override {
2929
buffer.resize(size);
3030
return 0;
3131
}

‎system/lib/wasmfs/pipe_backend.h‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -46,10 +46,10 @@ class PipeFile : public DataFile {
4646

4747
int flush() override { return 0; }
4848

49-
size_t getSize() override { return data->size(); }
49+
off_t getSize() override { return data->size(); }
5050

5151
// TODO: Should this return an error?
52-
int setSize(size_t size) override { return 0; }
52+
int setSize(off_t size) override { return 0; }
5353

5454
public:
5555
// PipeFiles do not have or need a backend. Pass NullBackend to the parent for

‎system/lib/wasmfs/proxied_async_js_impl_backend.h‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -66,7 +66,7 @@ void _wasmfs_jsimpl_async_read(em_proxying_ctx* ctx,
6666
void _wasmfs_jsimpl_async_get_size(em_proxying_ctx* ctx,
6767
js_index_t backend,
6868
js_index_t index,
69-
size_t* result);
69+
off_t* result);
7070
}
7171

7272
namespace wasmfs {
@@ -108,16 +108,16 @@ class ProxiedAsyncJSImplFile : public DataFile {
108108

109109
int flush() override { return 0; }
110110

111-
size_t getSize() override {
112-
size_t result;
111+
off_t getSize() override {
112+
off_t result;
113113
proxy([&](auto ctx) {
114114
_wasmfs_jsimpl_async_get_size(
115115
ctx.ctx, getBackendIndex(), getFileIndex(), &result);
116116
});
117117
return result;
118118
}
119119

120-
int setSize(size_t size) override {
120+
int setSize(off_t size) override {
121121
WASMFS_UNREACHABLE("TODO: ProxiedAsyncJSImplFile setSize");
122122
}
123123

‎system/lib/wasmfs/special_files.cpp‎

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -29,8 +29,8 @@ class NullFile : public DataFile {
2929
ssize_t read(uint8_t* buf, size_t len, off_t offset) override { return 0; }
3030

3131
int flush() override { return 0; }
32-
size_t getSize() override { return 0; }
33-
int setSize(size_t size) override { return -EPERM; }
32+
off_t getSize() override { return 0; }
33+
int setSize(off_t size) override { return -EPERM; }
3434

3535
public:
3636
NullFile() : DataFile(S_IRUGO | S_IWUGO, NullBackend, S_IFCHR) {}
@@ -50,8 +50,8 @@ class StdinFile : public DataFile {
5050
};
5151

5252
int flush() override { return 0; }
53-
size_t getSize() override { return 0; }
54-
int setSize(size_t size) override { return -EPERM; }
53+
off_t getSize() override { return 0; }
54+
int setSize(off_t size) override { return -EPERM; }
5555

5656
public:
5757
StdinFile() : DataFile(S_IRUGO, NullBackend, S_IFCHR) { seekable = false; }
@@ -78,8 +78,8 @@ class WritingStdFile : public DataFile {
7878
return 0;
7979
}
8080

81-
size_t getSize() override { return 0; }
82-
int setSize(size_t size) override { return -EPERM; }
81+
off_t getSize() override { return 0; }
82+
int setSize(off_t size) override { return -EPERM; }
8383

8484
ssize_t writeToJS(const uint8_t* buf,
8585
size_t len,
@@ -152,8 +152,8 @@ class RandomFile : public DataFile {
152152
};
153153

154154
int flush() override { return 0; }
155-
size_t getSize() override { return 0; }
156-
int setSize(size_t size) override { return -EPERM; }
155+
off_t getSize() override { return 0; }
156+
int setSize(off_t size) override { return -EPERM; }
157157

158158
public:
159159
RandomFile() : DataFile(S_IRUGO, NullBackend, S_IFCHR) { seekable = false; }

0 commit comments

Comments
 (0)