From d09310e9b817f3dfafcdbcf5090d990f6d8bacfb Mon Sep 17 00:00:00 2001 From: Hood Chatham Date: Wed, 11 Dec 2024 15:35:53 +0100 Subject: [PATCH 1/8] Return EISDIR when trying to create a path ending in / with open If we open a path that in a / and we're about to create a file, instead return EISDIR. --- src/library_fs.js | 3 +++ test/fs/test_fs_eisdir.c | 36 ++++++++++++++++++++++++++++++++++++ test/test_core.py | 10 ++++++++++ 3 files changed, 49 insertions(+) create mode 100644 test/fs/test_fs_eisdir.c diff --git a/src/library_fs.js b/src/library_fs.js index 68c53ad670bb5..7fbdedeb1449c 100644 --- a/src/library_fs.js +++ b/src/library_fs.js @@ -1053,6 +1053,7 @@ FS.staticInit(); mode = 0; } var node; + var isDirPath = path.endsWith("/"); if (typeof path == 'object') { node = path; } else { @@ -1074,6 +1075,8 @@ FS.staticInit(); if ((flags & {{{ cDefs.O_EXCL }}})) { throw new FS.ErrnoError({{{ cDefs.EEXIST }}}); } + } else if(isDirPath) { + throw new FS.ErrnoError({{{ cDefs.EISDIR }}}); } else { // node doesn't exist, try to create it node = FS.mknod(path, mode, 0); diff --git a/test/fs/test_fs_eisdir.c b/test/fs/test_fs_eisdir.c new file mode 100644 index 0000000000000..d2a73d6c39b72 --- /dev/null +++ b/test/fs/test_fs_eisdir.c @@ -0,0 +1,36 @@ +#include +#include +#include +#include +#include +#include +#include + +#if defined(__EMSCRIPTEN__) +#include +#endif + +void makedir(const char *dir) { + int rtn = mkdir(dir, 0777); + assert(rtn == 0); +} + +void changedir(const char *dir) { + int rtn = chdir(dir); + assert(rtn == 0); +} + +void setup() { +#if defined(__EMSCRIPTEN__) && defined(NODEFS) + makedir("working"); + EM_ASM(FS.mount(NODEFS, { root: '.' }, 'working')); + changedir("working"); +#endif +} + +int main() { + setup(); + open("./does-not-exist/", O_CREAT); + assert(errno == EISDIR); + printf("success\n"); +} diff --git a/test/test_core.py b/test/test_core.py index d3fff94c5a8ec..ed790827069a0 100644 --- a/test/test_core.py +++ b/test/test_core.py @@ -5900,6 +5900,16 @@ def test_fs_rename_on_existing(self, args): self.set_setting('FORCE_FILESYSTEM') self.do_runf('fs/test_fs_rename_on_existing.c', 'success', emcc_args=args) + @parameterized({ + '': ([],), + 'nodefs': (['-DNODEFS', '-lnodefs.js'],), + 'noderawfs': (['-sNODERAWFS'],) + }) + def test_fs_eisdir(self, args): + if self.get_setting('WASMFS'): + self.set_setting('FORCE_FILESYSTEM') + self.do_runf('fs/test_fs_eisdir.c', 'success', emcc_args=args) + def test_sigalrm(self): self.do_runf('test_sigalrm.c', 'Received alarm!') self.set_setting('EXIT_RUNTIME') From 7503f6f86c147c266b8590c34ed5b1fc0305df32 Mon Sep 17 00:00:00 2001 From: Hood Chatham Date: Thu, 12 Dec 2024 11:24:47 +0100 Subject: [PATCH 2/8] Fix --- src/library_fs.js | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/library_fs.js b/src/library_fs.js index 7fbdedeb1449c..c990524931688 100644 --- a/src/library_fs.js +++ b/src/library_fs.js @@ -1053,10 +1053,11 @@ FS.staticInit(); mode = 0; } var node; - var isDirPath = path.endsWith("/"); + var isDirPath; if (typeof path == 'object') { node = path; } else { + isDirPath = path.endsWith("/"); // noent_okay makes it so that if the final component of the path // doesn't exist, lookupPath returns `node: undefined`. `path` will be // updated to point to the target of all symlinks. From 07a00d1cf516b45ec46e8cb144cea2fcd58e50f4 Mon Sep 17 00:00:00 2001 From: Hood Chatham Date: Sat, 14 Dec 2024 11:14:48 +0100 Subject: [PATCH 3/8] Apply formatting changes --- src/library_fs.js | 2 +- test/fs/test_fs_eisdir.c | 8 ++++---- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/src/library_fs.js b/src/library_fs.js index c990524931688..971bfcd73c2d5 100644 --- a/src/library_fs.js +++ b/src/library_fs.js @@ -1076,7 +1076,7 @@ FS.staticInit(); if ((flags & {{{ cDefs.O_EXCL }}})) { throw new FS.ErrnoError({{{ cDefs.EEXIST }}}); } - } else if(isDirPath) { + } else if (isDirPath) { throw new FS.ErrnoError({{{ cDefs.EISDIR }}}); } else { // node doesn't exist, try to create it diff --git a/test/fs/test_fs_eisdir.c b/test/fs/test_fs_eisdir.c index d2a73d6c39b72..0d2a16421ab4a 100644 --- a/test/fs/test_fs_eisdir.c +++ b/test/fs/test_fs_eisdir.c @@ -29,8 +29,8 @@ void setup() { } int main() { - setup(); - open("./does-not-exist/", O_CREAT); - assert(errno == EISDIR); - printf("success\n"); + setup(); + open("./does-not-exist/", O_CREAT); + assert(errno == EISDIR); + printf("success\n"); } From 2162b247ace49f2a2c3ce43657f04a797cf5e234 Mon Sep 17 00:00:00 2001 From: Hood Chatham Date: Wed, 18 Dec 2024 11:34:50 +0100 Subject: [PATCH 4/8] Combine test_fs_enotdir with test_fs_eisdir --- test/fs/test_enotdir.c | 17 ----------------- test/fs/test_enotdir.out | 1 - test/fs/test_fs_eisdir.c | 36 ------------------------------------ test/fs/test_fs_enotdir.c | 23 +++++++++++++++++++++++ test/test_core.py | 2 +- 5 files changed, 24 insertions(+), 55 deletions(-) delete mode 100644 test/fs/test_enotdir.c delete mode 100644 test/fs/test_enotdir.out delete mode 100644 test/fs/test_fs_eisdir.c create mode 100644 test/fs/test_fs_enotdir.c diff --git a/test/fs/test_enotdir.c b/test/fs/test_enotdir.c deleted file mode 100644 index 0f9bb5f4509ad..0000000000000 --- a/test/fs/test_enotdir.c +++ /dev/null @@ -1,17 +0,0 @@ -#include -#include -#include -#include -#include -#include - -int main() { - { - int src_fd = open("file", O_CREAT | O_WRONLY, 0777); - close(src_fd); - } - { - int target_fd = mkdir("file/blah", 0777); - printf("target_fd: %d, errno: %d %s\n", target_fd, errno, strerror(errno)); - } -} diff --git a/test/fs/test_enotdir.out b/test/fs/test_enotdir.out deleted file mode 100644 index 55c2464fc101c..0000000000000 --- a/test/fs/test_enotdir.out +++ /dev/null @@ -1 +0,0 @@ -target_fd: -1, errno: 54 Not a directory diff --git a/test/fs/test_fs_eisdir.c b/test/fs/test_fs_eisdir.c deleted file mode 100644 index 0d2a16421ab4a..0000000000000 --- a/test/fs/test_fs_eisdir.c +++ /dev/null @@ -1,36 +0,0 @@ -#include -#include -#include -#include -#include -#include -#include - -#if defined(__EMSCRIPTEN__) -#include -#endif - -void makedir(const char *dir) { - int rtn = mkdir(dir, 0777); - assert(rtn == 0); -} - -void changedir(const char *dir) { - int rtn = chdir(dir); - assert(rtn == 0); -} - -void setup() { -#if defined(__EMSCRIPTEN__) && defined(NODEFS) - makedir("working"); - EM_ASM(FS.mount(NODEFS, { root: '.' }, 'working')); - changedir("working"); -#endif -} - -int main() { - setup(); - open("./does-not-exist/", O_CREAT); - assert(errno == EISDIR); - printf("success\n"); -} diff --git a/test/fs/test_fs_enotdir.c b/test/fs/test_fs_enotdir.c new file mode 100644 index 0000000000000..4f08dcfaa9088 --- /dev/null +++ b/test/fs/test_fs_enotdir.c @@ -0,0 +1,23 @@ +#include +#include +#include +#include +#include +#include +#include + +int main() { + { + int src_fd = open("file", O_CREAT | O_WRONLY, 0777); + assert(src_fd >= 0); + assert(close(src_fd) == 0); + } + { + assert(mkdir("file/blah", 0777) == -1); + assert(errno == ENOTDIR); + } + { + assert(open("./does-not-exist/", O_CREAT) == -1); + assert(errno == EISDIR); + } +} diff --git a/test/test_core.py b/test/test_core.py index 449050a37d455..30fe2e7104ccb 100644 --- a/test/test_core.py +++ b/test/test_core.py @@ -5764,7 +5764,7 @@ def test_fs_emptyPath(self): @crossplatform @also_with_noderawfs def test_fs_enotdir(self): - self.do_run_in_out_file_test('fs/test_enotdir.c') + self.do_runf('fs/test_fs_enotdir.c', 'success', emcc_args=args) @also_with_noderawfs def test_fs_append(self): From 68d1f7e861c5924367f5e3a343ada7302a27c69a Mon Sep 17 00:00:00 2001 From: Hood Chatham Date: Wed, 18 Dec 2024 11:41:12 +0100 Subject: [PATCH 5/8] use nodefs_both decorator --- test/fs/test_fs_enotdir.c | 1 + test/test_core.py | 4 ++-- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/test/fs/test_fs_enotdir.c b/test/fs/test_fs_enotdir.c index 4f08dcfaa9088..e221ed8e59304 100644 --- a/test/fs/test_fs_enotdir.c +++ b/test/fs/test_fs_enotdir.c @@ -20,4 +20,5 @@ int main() { assert(open("./does-not-exist/", O_CREAT) == -1); assert(errno == EISDIR); } + printf("success\n"); } diff --git a/test/test_core.py b/test/test_core.py index 719dd44465fd5..4672ee4149c99 100644 --- a/test/test_core.py +++ b/test/test_core.py @@ -5756,9 +5756,9 @@ def test_fs_emptyPath(self): @no_windows('https://github.com/emscripten-core/emscripten/issues/8882') @crossplatform - @also_with_noderawfs + @also_with_nodefs_both def test_fs_enotdir(self): - self.do_runf('fs/test_fs_enotdir.c', 'success', emcc_args=args) + self.do_runf('fs/test_fs_enotdir.c', 'success') @also_with_noderawfs def test_fs_append(self): From 071c87dbe6265f8c73fed88536eed6857cff9bf5 Mon Sep 17 00:00:00 2001 From: Hood Chatham Date: Wed, 18 Dec 2024 19:20:49 +0100 Subject: [PATCH 6/8] use also_with_nodefs_both decorator --- test/test_core.py | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/test/test_core.py b/test/test_core.py index 4672ee4149c99..12cb49b6a01b2 100644 --- a/test/test_core.py +++ b/test/test_core.py @@ -5858,15 +5858,11 @@ def test_fs_rename_on_existing(self): self.set_setting('FORCE_FILESYSTEM') self.do_runf('fs/test_fs_rename_on_existing.c', 'success') - @parameterized({ - '': ([],), - 'nodefs': (['-DNODEFS', '-lnodefs.js'],), - 'noderawfs': (['-sNODERAWFS'],) - }) + @also_with_nodefs_both def test_fs_eisdir(self, args): if self.get_setting('WASMFS'): self.set_setting('FORCE_FILESYSTEM') - self.do_runf('fs/test_fs_eisdir.c', 'success', emcc_args=args) + self.do_runf('fs/test_fs_eisdir.c', 'success') def test_sigalrm(self): self.do_runf('test_sigalrm.c', 'Received alarm!') From 04d6ffa1768798dbca3db6f982c22ba9897b36ac Mon Sep 17 00:00:00 2001 From: Hood Chatham Date: Wed, 18 Dec 2024 19:21:31 +0100 Subject: [PATCH 7/8] Remove other test --- test/test_core.py | 6 ------ 1 file changed, 6 deletions(-) diff --git a/test/test_core.py b/test/test_core.py index 12cb49b6a01b2..8aa390e4f2824 100644 --- a/test/test_core.py +++ b/test/test_core.py @@ -5858,12 +5858,6 @@ 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_eisdir(self, args): - if self.get_setting('WASMFS'): - self.set_setting('FORCE_FILESYSTEM') - self.do_runf('fs/test_fs_eisdir.c', 'success') - def test_sigalrm(self): self.do_runf('test_sigalrm.c', 'Received alarm!') self.do_runf('test_sigalrm.c', 'Received alarm!', emcc_args=['-sEXIT_RUNTIME']) From 3c470ae510caffc93d77ec7112d231ee90dccf2d Mon Sep 17 00:00:00 2001 From: Hood Chatham Date: Wed, 18 Dec 2024 20:58:19 +0100 Subject: [PATCH 8/8] Skip rawfs test on macos --- test/test_core.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/test/test_core.py b/test/test_core.py index 8aa390e4f2824..37e734e75ecf2 100644 --- a/test/test_core.py +++ b/test/test_core.py @@ -5758,6 +5758,8 @@ def test_fs_emptyPath(self): @crossplatform @also_with_nodefs_both def test_fs_enotdir(self): + if MACOS and '-DNODERAWFS' in self.emcc_args: + self.skipTest('BSD libc sets a different errno') self.do_runf('fs/test_fs_enotdir.c', 'success') @also_with_noderawfs