Skip to content

Commit

Permalink
Make open O_CREATE mode 0 work (#23137)
Browse files Browse the repository at this point in the history
In nodefs we first create the node with permissions 0 and then try to
open it. The open fails because we don't have read permissions.

The following (18) test expectation files were updated by
running the tests with `--rebaseline`:

```
other/codesize/test_codesize_cxx_ctors1.gzsize: 8351 => 8407 [+56 bytes / +0.67%]
other/codesize/test_codesize_cxx_ctors1.jssize: 20343 => 20486 [+143 bytes / +0.70%]
other/codesize/test_codesize_cxx_ctors2.gzsize: 8334 => 8390 [+56 bytes / +0.67%]
other/codesize/test_codesize_cxx_ctors2.jssize: 20311 => 20454 [+143 bytes / +0.70%]
other/codesize/test_codesize_cxx_except.gzsize: 9356 => 9411 [+55 bytes / +0.59%]
other/codesize/test_codesize_cxx_except.jssize: 24112 => 24255 [+143 bytes / +0.59%]
other/codesize/test_codesize_cxx_except_wasm.gzsize: 8321 => 8373 [+52 bytes / +0.62%]
other/codesize/test_codesize_cxx_except_wasm.jssize: 20236 => 20379 [+143 bytes / +0.71%]
other/codesize/test_codesize_cxx_except_wasm_exnref.gzsize: 8321 => 8373 [+52 bytes / +0.62%]
other/codesize/test_codesize_cxx_except_wasm_exnref.jssize: 20236 => 20379 [+143 bytes / +0.71%]
other/codesize/test_codesize_cxx_lto.gzsize: 8347 => 8404 [+57 bytes / +0.68%]
other/codesize/test_codesize_cxx_lto.jssize: 20367 => 20510 [+143 bytes / +0.70%]
other/codesize/test_codesize_cxx_mangle.gzsize: 9359 => 9416 [+57 bytes / +0.61%]
other/codesize/test_codesize_cxx_mangle.jssize: 24112 => 24255 [+143 bytes / +0.59%]
other/codesize/test_codesize_cxx_noexcept.gzsize: 8351 => 8407 [+56 bytes / +0.67%]
other/codesize/test_codesize_cxx_noexcept.jssize: 20343 => 20486 [+143 bytes / +0.70%]
other/codesize/test_codesize_files_js_fs.gzsize: 7654 => 7702 [+48 bytes / +0.63%]
other/codesize/test_codesize_files_js_fs.jssize: 18838 => 18980 [+142 bytes / +0.75%]

Average change: +0.66% (+0.59% - +0.75%)
```
  • Loading branch information
hoodmane authored Dec 19, 2024
1 parent 971d4f6 commit 89c946e
Show file tree
Hide file tree
Showing 21 changed files with 50 additions and 19 deletions.
8 changes: 7 additions & 1 deletion src/library_fs.js
Original file line number Diff line number Diff line change
Expand Up @@ -1087,7 +1087,10 @@ FS.staticInit();
throw new FS.ErrnoError({{{ cDefs.EISDIR }}});
} else {
// node doesn't exist, try to create it
node = FS.mknod(path, mode, 0);
// Ignore the permission bits here to ensure we can `open` this new
// file below. We use chmod below the apply the permissions once the
// file is open.
node = FS.mknod(path, mode | 0o777, 0);
created = true;
}
}
Expand Down Expand Up @@ -1137,6 +1140,9 @@ FS.staticInit();
if (stream.stream_ops.open) {
stream.stream_ops.open(stream);
}
if (created) {
FS.chmod(node, mode & 0o777);
}
#if expectToReceiveOnModule('logReadFiles')
if (Module['logReadFiles'] && !(flags & {{{ cDefs.O_WRONLY}}})) {
if (!(path in FS.readFiles)) {
Expand Down
21 changes: 21 additions & 0 deletions test/fs/test_fs_open_no_permissions.c
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
#include <stdio.h>
#include <unistd.h>
#include <assert.h>
#include <fcntl.h>
#include <string.h>
#include <sys/stat.h>
#include <assert.h>
#include <errno.h>
#include <string.h>

// TODO: Combine this with test_fcntl_open.c. First requires fixing
// test_fcntl_open.c on noderawfs.
int main() {
int res = open("a", O_CREAT, 0);
printf("error: %s\n", strerror(errno));
assert(res >= 0);
struct stat st;
assert(stat("a", &st) == 0);
assert((st.st_mode & 0777) == 0);
printf("success\n");
}
2 changes: 1 addition & 1 deletion test/other/codesize/test_codesize_cxx_ctors1.gzsize
Original file line number Diff line number Diff line change
@@ -1 +1 @@
8351
8407
2 changes: 1 addition & 1 deletion test/other/codesize/test_codesize_cxx_ctors1.jssize
Original file line number Diff line number Diff line change
@@ -1 +1 @@
20343
20486
2 changes: 1 addition & 1 deletion test/other/codesize/test_codesize_cxx_ctors2.gzsize
Original file line number Diff line number Diff line change
@@ -1 +1 @@
8334
8390
2 changes: 1 addition & 1 deletion test/other/codesize/test_codesize_cxx_ctors2.jssize
Original file line number Diff line number Diff line change
@@ -1 +1 @@
20311
20454
2 changes: 1 addition & 1 deletion test/other/codesize/test_codesize_cxx_except.gzsize
Original file line number Diff line number Diff line change
@@ -1 +1 @@
9356
9411
2 changes: 1 addition & 1 deletion test/other/codesize/test_codesize_cxx_except.jssize
Original file line number Diff line number Diff line change
@@ -1 +1 @@
24112
24255
2 changes: 1 addition & 1 deletion test/other/codesize/test_codesize_cxx_except_wasm.gzsize
Original file line number Diff line number Diff line change
@@ -1 +1 @@
8321
8373
2 changes: 1 addition & 1 deletion test/other/codesize/test_codesize_cxx_except_wasm.jssize
Original file line number Diff line number Diff line change
@@ -1 +1 @@
20236
20379
Original file line number Diff line number Diff line change
@@ -1 +1 @@
8321
8373
Original file line number Diff line number Diff line change
@@ -1 +1 @@
20236
20379
2 changes: 1 addition & 1 deletion test/other/codesize/test_codesize_cxx_lto.gzsize
Original file line number Diff line number Diff line change
@@ -1 +1 @@
8347
8404
2 changes: 1 addition & 1 deletion test/other/codesize/test_codesize_cxx_lto.jssize
Original file line number Diff line number Diff line change
@@ -1 +1 @@
20367
20510
2 changes: 1 addition & 1 deletion test/other/codesize/test_codesize_cxx_mangle.gzsize
Original file line number Diff line number Diff line change
@@ -1 +1 @@
9359
9416
2 changes: 1 addition & 1 deletion test/other/codesize/test_codesize_cxx_mangle.jssize
Original file line number Diff line number Diff line change
@@ -1 +1 @@
24112
24255
2 changes: 1 addition & 1 deletion test/other/codesize/test_codesize_cxx_noexcept.gzsize
Original file line number Diff line number Diff line change
@@ -1 +1 @@
8351
8407
2 changes: 1 addition & 1 deletion test/other/codesize/test_codesize_cxx_noexcept.jssize
Original file line number Diff line number Diff line change
@@ -1 +1 @@
20343
20486
2 changes: 1 addition & 1 deletion test/other/codesize/test_codesize_files_js_fs.gzsize
Original file line number Diff line number Diff line change
@@ -1 +1 @@
7654
7702
2 changes: 1 addition & 1 deletion test/other/codesize/test_codesize_files_js_fs.jssize
Original file line number Diff line number Diff line change
@@ -1 +1 @@
18838
18980
4 changes: 4 additions & 0 deletions test/test_core.py
Original file line number Diff line number Diff line change
Expand Up @@ -5858,6 +5858,10 @@ def test_fs_rename_on_existing(self):
self.set_setting('FORCE_FILESYSTEM')
self.do_runf('fs/test_fs_rename_on_existing.c', 'success')

@also_with_nodefs_both
def test_fs_open_no_permissions(self):
self.do_runf('fs/test_fs_open_no_permissions.c', 'success')

@also_with_nodefs_both
@crossplatform
@no_windows('https://github.com/emscripten-core/emscripten/issues/8882')
Expand Down

0 comments on commit 89c946e

Please sign in to comment.