From 6becbf1c4a0076c3a6d542a10410692ebe403252 Mon Sep 17 00:00:00 2001 From: Hood Chatham Date: Thu, 14 Nov 2024 12:30:33 +0100 Subject: [PATCH 1/8] Add nodefs handling for directories that contain exotic entries --- src/library_syscall.js | 13 ++++++++++++- test/test_core.py | 1 + 2 files changed, 13 insertions(+), 1 deletion(-) diff --git a/src/library_syscall.js b/src/library_syscall.js index 14e00b3c265c4..f6a77423adaef 100644 --- a/src/library_syscall.js +++ b/src/library_syscall.js @@ -717,7 +717,18 @@ var SyscallsLibrary = { type = 4; // DT_DIR } else { - var child = FS.lookupNode(stream.node, name); + var child; + try { + child = FS.lookupNode(stream.node, name); + } catch (e) { + // If the entry is not a directory, file, or symlink, nodefs + // lookupNode will raise EINVAL. Skip these and continue. + if (e?.errno === {{{ cDefs.EINVAL }}}) { + idx += 1; + continue; + } + throw e; + } id = child.id; type = FS.isChrdev(child.mode) ? 2 : // DT_CHR, character device. FS.isDir(child.mode) ? 4 : // DT_DIR, directory. diff --git a/test/test_core.py b/test/test_core.py index ba8c3d64e2e1e..448f7934ee6b6 100644 --- a/test/test_core.py +++ b/test/test_core.py @@ -5762,6 +5762,7 @@ def test_fs_nodefs_readdir(self): # externally setup an existing folder structure: existing/a if self.get_setting('WASMFS'): self.set_setting('FORCE_FILESYSTEM') + os.mkfifo(os.path.join(self.working_dir, 'named_pipe')) os.makedirs(os.path.join(self.working_dir, 'existing', 'a')) self.emcc_args += ['-lnodefs.js'] self.do_runf('fs/test_nodefs_readdir.c', 'success') From e82f062850aef185e8887a064a986627013ebabb Mon Sep 17 00:00:00 2001 From: Hood Chatham Date: Sat, 16 Nov 2024 11:38:27 +0100 Subject: [PATCH 2/8] Add @crossplatform and restrict mkfifo to unix, switch to do_run_in_out_file --- test/fs/test_nodefs_readdir.out | 13 +++++++++++++ test/test_core.py | 8 ++++++-- 2 files changed, 19 insertions(+), 2 deletions(-) create mode 100644 test/fs/test_nodefs_readdir.out diff --git a/test/fs/test_nodefs_readdir.out b/test/fs/test_nodefs_readdir.out new file mode 100644 index 0000000000000..293d117dc7251 --- /dev/null +++ b/test/fs/test_nodefs_readdir.out @@ -0,0 +1,13 @@ +listing contents of dir=/ +. +.. +tmp +home +dev +proc +listing contents of dir=/working +existing +stdout +test_nodefs_readdir.js +test_nodefs_readdir.wasm +success diff --git a/test/test_core.py b/test/test_core.py index 448f7934ee6b6..0d4b1b3b3d44c 100644 --- a/test/test_core.py +++ b/test/test_core.py @@ -5757,15 +5757,19 @@ def test_fs_nodefs_nofollow(self): self.emcc_args += ['-lnodefs.js'] self.do_runf('fs/test_nodefs_nofollow.c', 'success') + @crossplatform @requires_node def test_fs_nodefs_readdir(self): # externally setup an existing folder structure: existing/a if self.get_setting('WASMFS'): self.set_setting('FORCE_FILESYSTEM') - os.mkfifo(os.path.join(self.working_dir, 'named_pipe')) + if not WINDOWS and not MACOS: + # Add an entry that isn't a directory, file, or link to test that we handle + # it correctly. + os.mkfifo(os.path.join(self.working_dir, 'named_pipe')) os.makedirs(os.path.join(self.working_dir, 'existing', 'a')) self.emcc_args += ['-lnodefs.js'] - self.do_runf('fs/test_nodefs_readdir.c', 'success') + self.do_run_in_out_file_test('fs/test_nodefs_readdir.c') @no_windows('no symlink support on windows') @requires_node From 65f3d78fed2d1786afe278ec20bbe13425d8a51c Mon Sep 17 00:00:00 2001 From: Hood Chatham Date: Sat, 16 Nov 2024 11:43:03 +0100 Subject: [PATCH 3/8] Switch for loop to while loop --- src/library_syscall.js | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/src/library_syscall.js b/src/library_syscall.js index f6a77423adaef..e393058f84123 100644 --- a/src/library_syscall.js +++ b/src/library_syscall.js @@ -701,9 +701,9 @@ var SyscallsLibrary = { var pos = 0; var off = FS.llseek(stream, 0, {{{ cDefs.SEEK_CUR }}}); - var idx = Math.floor(off / struct_size); - - while (idx < stream.getdents.length && pos + struct_size <= count) { + var startIdx = Math.floor(off / struct_size); + var endIdx = Math.min(stream.getdents.length, startIdx + Math.floor(count/struct_size)) + for (var idx = startIdx; idx < endIdx; idx++) { var id; var type; var name = stream.getdents[idx]; @@ -724,7 +724,6 @@ var SyscallsLibrary = { // If the entry is not a directory, file, or symlink, nodefs // lookupNode will raise EINVAL. Skip these and continue. if (e?.errno === {{{ cDefs.EINVAL }}}) { - idx += 1; continue; } throw e; @@ -744,7 +743,6 @@ var SyscallsLibrary = { {{{ makeSetValue('dirp + pos', C_STRUCTS.dirent.d_type, 'type', 'i8') }}}; stringToUTF8(name, dirp + pos + {{{ C_STRUCTS.dirent.d_name }}}, 256); pos += struct_size; - idx += 1; } FS.llseek(stream, idx * struct_size, {{{ cDefs.SEEK_SET }}}); return pos; From 03b5e7283dc8d3afebdef07481eca2a23ecc4b2f Mon Sep 17 00:00:00 2001 From: Hood Chatham Date: Tue, 26 Nov 2024 13:42:53 +0100 Subject: [PATCH 4/8] Fix test --- test/fs/test_nodefs_readdir.out | 13 ------------- test/test_core.py | 17 ++++++++++++++++- 2 files changed, 16 insertions(+), 14 deletions(-) delete mode 100644 test/fs/test_nodefs_readdir.out diff --git a/test/fs/test_nodefs_readdir.out b/test/fs/test_nodefs_readdir.out deleted file mode 100644 index 293d117dc7251..0000000000000 --- a/test/fs/test_nodefs_readdir.out +++ /dev/null @@ -1,13 +0,0 @@ -listing contents of dir=/ -. -.. -tmp -home -dev -proc -listing contents of dir=/working -existing -stdout -test_nodefs_readdir.js -test_nodefs_readdir.wasm -success diff --git a/test/test_core.py b/test/test_core.py index 0d4b1b3b3d44c..24a9eb62f7f80 100644 --- a/test/test_core.py +++ b/test/test_core.py @@ -5769,7 +5769,22 @@ def test_fs_nodefs_readdir(self): os.mkfifo(os.path.join(self.working_dir, 'named_pipe')) os.makedirs(os.path.join(self.working_dir, 'existing', 'a')) self.emcc_args += ['-lnodefs.js'] - self.do_run_in_out_file_test('fs/test_nodefs_readdir.c') + result = self.do_runf('fs/test_nodefs_readdir.c') + assert result.endswith("success\n") + + result = result.removesuffix("success\n") + split = result.split("listing contents of ") + root_result = split[1].splitlines() + working_result = split[2].splitlines() + if self.get_setting('WASMFS'): + self.assertEqual(root_result, ['dir=/', '.', '..', 'dev', 'tmp']) + else: + self.assertEqual(root_result, ['dir=/', '.', '..', 'tmp', 'home', 'dev', 'proc']) + if self.get_setting('WASMFS'): + self.assertEqual(working_result, ['dir=/working', '.', '..', 'existing', 'named_pipe', 'stdout', 'test_nodefs_readdir.js', 'test_nodefs_readdir.wasm']) + else: + self.assertEqual(working_result, ['dir=/working', 'existing', 'stdout', 'test_nodefs_readdir.js', 'test_nodefs_readdir.wasm']) + @no_windows('no symlink support on windows') @requires_node From 8186ce5fa26faef17999cd061e89ce0ba1f7729d Mon Sep 17 00:00:00 2001 From: Hood Chatham Date: Tue, 26 Nov 2024 13:44:50 +0100 Subject: [PATCH 5/8] Fix flake8 --- test/test_core.py | 1 - 1 file changed, 1 deletion(-) diff --git a/test/test_core.py b/test/test_core.py index 24a9eb62f7f80..bfeeb391501df 100644 --- a/test/test_core.py +++ b/test/test_core.py @@ -5785,7 +5785,6 @@ def test_fs_nodefs_readdir(self): else: self.assertEqual(working_result, ['dir=/working', 'existing', 'stdout', 'test_nodefs_readdir.js', 'test_nodefs_readdir.wasm']) - @no_windows('no symlink support on windows') @requires_node def test_fs_noderawfs_nofollow(self): From 7b2a320cb365823c694363b35c4fe205afe89f24 Mon Sep 17 00:00:00 2001 From: Hood Chatham Date: Tue, 26 Nov 2024 14:47:52 +0100 Subject: [PATCH 6/8] Fix test again --- test/test_core.py | 17 +++-------------- 1 file changed, 3 insertions(+), 14 deletions(-) diff --git a/test/test_core.py b/test/test_core.py index bfeeb391501df..11a4f3822e8e0 100644 --- a/test/test_core.py +++ b/test/test_core.py @@ -5769,21 +5769,10 @@ def test_fs_nodefs_readdir(self): os.mkfifo(os.path.join(self.working_dir, 'named_pipe')) os.makedirs(os.path.join(self.working_dir, 'existing', 'a')) self.emcc_args += ['-lnodefs.js'] - result = self.do_runf('fs/test_nodefs_readdir.c') - assert result.endswith("success\n") - - result = result.removesuffix("success\n") - split = result.split("listing contents of ") - root_result = split[1].splitlines() - working_result = split[2].splitlines() - if self.get_setting('WASMFS'): - self.assertEqual(root_result, ['dir=/', '.', '..', 'dev', 'tmp']) - else: - self.assertEqual(root_result, ['dir=/', '.', '..', 'tmp', 'home', 'dev', 'proc']) + suffix = "" if self.get_setting('WASMFS'): - self.assertEqual(working_result, ['dir=/working', '.', '..', 'existing', 'named_pipe', 'stdout', 'test_nodefs_readdir.js', 'test_nodefs_readdir.wasm']) - else: - self.assertEqual(working_result, ['dir=/working', 'existing', 'stdout', 'test_nodefs_readdir.js', 'test_nodefs_readdir.wasm']) + suffix = ".wasm" + self.do_run_in_out_file_test('fs/test_nodefs_readdir.c', out_suffix=suffix) @no_windows('no symlink support on windows') @requires_node From f84df658e04be48f6ae2e4b6e7b591a2ea790a29 Mon Sep 17 00:00:00 2001 From: Hood Chatham Date: Wed, 27 Nov 2024 09:04:26 +0100 Subject: [PATCH 7/8] Address review comments --- test/fs/test_nodefs_readdir.out | 13 +++++++++++++ test/fs/test_nodefs_readdir.wasmfs.out | 14 ++++++++++++++ test/test_core.py | 10 +++++----- 3 files changed, 32 insertions(+), 5 deletions(-) create mode 100644 test/fs/test_nodefs_readdir.out create mode 100644 test/fs/test_nodefs_readdir.wasmfs.out diff --git a/test/fs/test_nodefs_readdir.out b/test/fs/test_nodefs_readdir.out new file mode 100644 index 0000000000000..293d117dc7251 --- /dev/null +++ b/test/fs/test_nodefs_readdir.out @@ -0,0 +1,13 @@ +listing contents of dir=/ +. +.. +tmp +home +dev +proc +listing contents of dir=/working +existing +stdout +test_nodefs_readdir.js +test_nodefs_readdir.wasm +success diff --git a/test/fs/test_nodefs_readdir.wasmfs.out b/test/fs/test_nodefs_readdir.wasmfs.out new file mode 100644 index 0000000000000..bf8f2ad9fa60a --- /dev/null +++ b/test/fs/test_nodefs_readdir.wasmfs.out @@ -0,0 +1,14 @@ +listing contents of dir=/ +. +.. +dev +tmp +listing contents of dir=/working +. +.. +existing +named_pipe +stdout +test_nodefs_readdir.js +test_nodefs_readdir.wasm +success diff --git a/test/test_core.py b/test/test_core.py index 11a4f3822e8e0..966f312be3be2 100644 --- a/test/test_core.py +++ b/test/test_core.py @@ -5763,15 +5763,15 @@ def test_fs_nodefs_readdir(self): # externally setup an existing folder structure: existing/a if self.get_setting('WASMFS'): self.set_setting('FORCE_FILESYSTEM') - if not WINDOWS and not MACOS: + if not WINDOWS: # Add an entry that isn't a directory, file, or link to test that we handle # it correctly. - os.mkfifo(os.path.join(self.working_dir, 'named_pipe')) - os.makedirs(os.path.join(self.working_dir, 'existing', 'a')) + os.mkfifo('named_pipe') + os.makedirs(os.path.join('existing', 'a')) self.emcc_args += ['-lnodefs.js'] - suffix = "" + suffix = '' if self.get_setting('WASMFS'): - suffix = ".wasm" + suffix = '.wasmfs' self.do_run_in_out_file_test('fs/test_nodefs_readdir.c', out_suffix=suffix) @no_windows('no symlink support on windows') From 97fc34c31e2c6e810f1897fd1721edb64e702c87 Mon Sep 17 00:00:00 2001 From: Hood Chatham Date: Thu, 28 Nov 2024 00:10:03 +0100 Subject: [PATCH 8/8] Fix test for wasm2js --- test/fs/test_nodefs_readdir.wasm2js.out | 12 ++++++++++++ test/test_core.py | 2 ++ 2 files changed, 14 insertions(+) create mode 100644 test/fs/test_nodefs_readdir.wasm2js.out diff --git a/test/fs/test_nodefs_readdir.wasm2js.out b/test/fs/test_nodefs_readdir.wasm2js.out new file mode 100644 index 0000000000000..cc2268c8bcb8a --- /dev/null +++ b/test/fs/test_nodefs_readdir.wasm2js.out @@ -0,0 +1,12 @@ +listing contents of dir=/ +. +.. +tmp +home +dev +proc +listing contents of dir=/working +existing +stdout +test_nodefs_readdir.js +success diff --git a/test/test_core.py b/test/test_core.py index d25adcbb72bdc..82db1dbd35ff9 100644 --- a/test/test_core.py +++ b/test/test_core.py @@ -5794,6 +5794,8 @@ def test_fs_nodefs_readdir(self): suffix = '' if self.get_setting('WASMFS'): suffix = '.wasmfs' + elif self.is_wasm2js(): + suffix = ".wasm2js" self.do_run_in_out_file_test('fs/test_nodefs_readdir.c', out_suffix=suffix) @requires_node