From 9a54975cf9b001918278086dc26406d2ec363791 Mon Sep 17 00:00:00 2001 From: Hood Chatham Date: Wed, 4 Dec 2024 21:28:35 +0100 Subject: [PATCH 1/4] Fix renaming a file on top of an existing file in memfs and nodefs --- src/library_memfs.js | 6 +++- src/library_nodefs.js | 3 ++ test/fs/test_rename_on_existing.c | 55 +++++++++++++++++++++++++++++++ test/test_core.py | 10 ++++++ 4 files changed, 73 insertions(+), 1 deletion(-) create mode 100644 test/fs/test_rename_on_existing.c diff --git a/src/library_memfs.js b/src/library_memfs.js index b4beccd08aadf..44c6e72e1d238 100644 --- a/src/library_memfs.js +++ b/src/library_memfs.js @@ -193,8 +193,8 @@ addToLibrary({ }, rename(old_node, new_dir, new_name) { // if we're overwriting a directory at new_name, make sure it's empty. + var new_node; if (FS.isDir(old_node.mode)) { - var new_node; try { new_node = FS.lookupNode(new_dir, new_name); } catch (e) { @@ -205,6 +205,10 @@ addToLibrary({ } } } + try { + new_node = FS.lookupNode(new_dir, new_name); + FS.hashRemoveNode(new_node); + } catch (e) {} // do the internal rewiring delete old_node.parent.contents[old_node.name]; old_node.parent.timestamp = Date.now() diff --git a/src/library_nodefs.js b/src/library_nodefs.js index 6d0544c8527cc..649946a20f267 100644 --- a/src/library_nodefs.js +++ b/src/library_nodefs.js @@ -196,6 +196,9 @@ addToLibrary({ rename(oldNode, newDir, newName) { var oldPath = NODEFS.realPath(oldNode); var newPath = PATH.join2(NODEFS.realPath(newDir), newName); + try { + FS.unlink(newPath); + } catch(e) {} NODEFS.tryFSOperation(() => fs.renameSync(oldPath, newPath)); oldNode.name = newName; }, diff --git a/test/fs/test_rename_on_existing.c b/test/fs/test_rename_on_existing.c new file mode 100644 index 0000000000000..065186ad2fa82 --- /dev/null +++ b/test/fs/test_rename_on_existing.c @@ -0,0 +1,55 @@ +#include +#include +#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); +} + +static void create_file(const char *path, const char *buffer) { + printf("creating: %s\n", path); + int fd = open(path, O_WRONLY | O_CREAT | O_EXCL, 0666); + printf("error: %s\n", strerror(errno)); + assert(fd >= 0); + + int err = write(fd, buffer, sizeof(char) * strlen(buffer)); + assert(err == (sizeof(char) * strlen(buffer))); + + close(fd); +} + + +void setup() { + makedir("working"); +#if defined(__EMSCRIPTEN__) && defined(NODEFS) + EM_ASM(FS.mount(NODEFS, { root: '.' }, 'working')); +#endif + changedir("working"); +} + +int main() { + setup(); + create_file("a", "abc"); + create_file("b", "xyz"); + assert(rename("a", "b") == 0); + assert(unlink("b") == 0); + create_file("b", "xyz"); + printf("success\n"); +} diff --git a/test/test_core.py b/test/test_core.py index 7511815c00a8e..4a85c3b8b99df 100644 --- a/test/test_core.py +++ b/test/test_core.py @@ -5935,6 +5935,16 @@ def test_fs_64bit(self): self.set_setting('FORCE_FILESYSTEM') self.do_runf('fs/test_64bit.c', 'success') + @parameterized({ + '': ([],), + 'nodefs': (['-DNODEFS', '-lnodefs.js'],), + 'noderawfs': (['-sNODERAWFS'],) + }) + def test_fs_rename_on_existing(self, args): + if self.get_setting('WASMFS'): + self.set_setting('FORCE_FILESYSTEM') + self.do_runf('fs/test_rename_on_existing.c', 'success', emcc_args=args) + def test_sigalrm(self): self.do_runf('test_sigalrm.c', 'Received alarm!') self.set_setting('EXIT_RUNTIME') From 905ac127fd406fc7fee0c40ed04d6e5d2dccf872 Mon Sep 17 00:00:00 2001 From: Hood Chatham Date: Wed, 4 Dec 2024 23:46:28 +0100 Subject: [PATCH 2/4] Rearrange memfs rename --- src/library_memfs.js | 18 +++++++----------- 1 file changed, 7 insertions(+), 11 deletions(-) diff --git a/src/library_memfs.js b/src/library_memfs.js index 44c6e72e1d238..e6e193f1a1ef4 100644 --- a/src/library_memfs.js +++ b/src/library_memfs.js @@ -192,23 +192,19 @@ addToLibrary({ return MEMFS.createNode(parent, name, mode, dev); }, rename(old_node, new_dir, new_name) { - // if we're overwriting a directory at new_name, make sure it's empty. var new_node; - if (FS.isDir(old_node.mode)) { - try { - new_node = FS.lookupNode(new_dir, new_name); - } catch (e) { - } - if (new_node) { + try { + new_node = FS.lookupNode(new_dir, new_name); + } catch (e) {} + if (new_node) { + if (FS.isDir(old_node.mode)) { + // if we're overwriting a directory at new_name, make sure it's empty. for (var i in new_node.contents) { throw new FS.ErrnoError({{{ cDefs.ENOTEMPTY }}}); } } - } - try { - new_node = FS.lookupNode(new_dir, new_name); FS.hashRemoveNode(new_node); - } catch (e) {} + } // do the internal rewiring delete old_node.parent.contents[old_node.name]; old_node.parent.timestamp = Date.now() From d446dc248acfffe231a2dbccfbdbf12453e8232c Mon Sep 17 00:00:00 2001 From: Hood Chatham Date: Thu, 5 Dec 2024 11:38:47 +0100 Subject: [PATCH 3/4] Match test file name --- .../{test_rename_on_existing.c => test_fs_rename_on_existing} | 0 test/test_core.py | 2 +- 2 files changed, 1 insertion(+), 1 deletion(-) rename test/fs/{test_rename_on_existing.c => test_fs_rename_on_existing} (100%) diff --git a/test/fs/test_rename_on_existing.c b/test/fs/test_fs_rename_on_existing similarity index 100% rename from test/fs/test_rename_on_existing.c rename to test/fs/test_fs_rename_on_existing diff --git a/test/test_core.py b/test/test_core.py index 4a85c3b8b99df..735e956ae7150 100644 --- a/test/test_core.py +++ b/test/test_core.py @@ -5943,7 +5943,7 @@ def test_fs_64bit(self): def test_fs_rename_on_existing(self, args): if self.get_setting('WASMFS'): self.set_setting('FORCE_FILESYSTEM') - self.do_runf('fs/test_rename_on_existing.c', 'success', emcc_args=args) + self.do_runf('fs/test_fs_rename_on_existing.c', 'success', emcc_args=args) def test_sigalrm(self): self.do_runf('test_sigalrm.c', 'Received alarm!') From ca54ac571cfdd6606251790fdfcc9659a3e5016f Mon Sep 17 00:00:00 2001 From: Hood Chatham Date: Thu, 5 Dec 2024 11:41:17 +0100 Subject: [PATCH 4/4] Move all of setup function into ifdef --- ...est_fs_rename_on_existing => test_fs_rename_on_existing.c} | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) rename test/fs/{test_fs_rename_on_existing => test_fs_rename_on_existing.c} (100%) diff --git a/test/fs/test_fs_rename_on_existing b/test/fs/test_fs_rename_on_existing.c similarity index 100% rename from test/fs/test_fs_rename_on_existing rename to test/fs/test_fs_rename_on_existing.c index 065186ad2fa82..8c32bc3ed1b49 100644 --- a/test/fs/test_fs_rename_on_existing +++ b/test/fs/test_fs_rename_on_existing.c @@ -37,11 +37,11 @@ static void create_file(const char *path, const char *buffer) { void setup() { - makedir("working"); #if defined(__EMSCRIPTEN__) && defined(NODEFS) + makedir("working"); EM_ASM(FS.mount(NODEFS, { root: '.' }, 'working')); -#endif changedir("working"); +#endif } int main() {