Correctly handle calls setting timestamp to 0 (#23310)

If someone calls `utimes("file", 0, 0)` the previous code I wrote would not adjust the time stamp.
emscripten-core · Jan 9, 2025 · 25dea46 · 25dea46
1 parent a5f3599
commit 25dea46
Show file tree

Hide file tree

Showing 23 changed files with 41 additions and 25 deletions.
diff --git a/src/library_memfs.js b/src/library_memfs.js
@@ -172,7 +172,7 @@ addToLibrary({
       },
       setattr(node, attr) {
         for (const key of ["mode", "atime", "mtime", "ctime"]) {
-          if (attr[key]) {
+          if (attr[key] != null) {
             node[key] = attr[key];
           }
         }

diff --git a/src/library_nodefs.js b/src/library_nodefs.js
@@ -166,9 +166,15 @@ addToLibrary({
             // update the common node structure mode as well
             node.mode = attr.mode;
           }
-          if (attr.atime || attr.mtime) {
-            var atime = attr.atime && new Date(attr.atime);
-            var mtime = attr.mtime && new Date(attr.mtime);
+          if (typeof (attr.atime ?? attr.mtime) === "number") {
+            // Unfortunately, we have to stat the current value if we don't want
+            // to change it. On top of that, since the times don't round trip
+            // this will only keep the value nearly unchanged not exactly
+            // unchanged. See:
+            // https://github.com/nodejs/node/issues/56492
+            var stat = () => fs.lstatSync(NODEFS.realPath(node));
+            var atime = new Date(attr.atime ?? stat().atime);
+            var mtime = new Date(attr.mtime ?? stat().mtime);
             fs.utimesSync(path, atime, mtime);
           }
           if (attr.size !== undefined) {

diff --git a/src/library_workerfs.js b/src/library_workerfs.js
@@ -91,7 +91,7 @@ addToLibrary({
       },
       setattr(node, attr) {
         for (const key of ["mode", "atime", "mtime", "ctime"]) {
-          if (attr[key]) {
+          if (attr[key] != null) {
             node[key] = attr[key];
           }
         }

diff --git a/test/other/codesize/test_codesize_cxx_ctors1.gzsize b/test/other/codesize/test_codesize_cxx_ctors1.gzsize
@@ -1 +1 @@
-8332
+8335
diff --git a/test/other/codesize/test_codesize_cxx_ctors1.jssize b/test/other/codesize/test_codesize_cxx_ctors1.jssize
@@ -1 +1 @@
-20255
+20261
diff --git a/test/other/codesize/test_codesize_cxx_ctors2.gzsize b/test/other/codesize/test_codesize_cxx_ctors2.gzsize
@@ -1 +1 @@
-8314
+8317
diff --git a/test/other/codesize/test_codesize_cxx_ctors2.jssize b/test/other/codesize/test_codesize_cxx_ctors2.jssize
@@ -1 +1 @@
-20223
+20229
diff --git a/test/other/codesize/test_codesize_cxx_except.gzsize b/test/other/codesize/test_codesize_cxx_except.gzsize
@@ -1 +1 @@
-9336
+9339
diff --git a/test/other/codesize/test_codesize_cxx_except.jssize b/test/other/codesize/test_codesize_cxx_except.jssize
@@ -1 +1 @@
-24023
+24029
diff --git a/test/other/codesize/test_codesize_cxx_except_wasm.gzsize b/test/other/codesize/test_codesize_cxx_except_wasm.gzsize
@@ -1 +1 @@
-8280
+8283
diff --git a/test/other/codesize/test_codesize_cxx_except_wasm.jssize b/test/other/codesize/test_codesize_cxx_except_wasm.jssize
@@ -1 +1 @@
-20148
+20154
diff --git a/test/other/codesize/test_codesize_cxx_except_wasm_exnref.gzsize b/test/other/codesize/test_codesize_cxx_except_wasm_exnref.gzsize
@@ -1 +1 @@
-8280
+8283
diff --git a/test/other/codesize/test_codesize_cxx_except_wasm_exnref.jssize b/test/other/codesize/test_codesize_cxx_except_wasm_exnref.jssize
@@ -1 +1 @@
-20148
+20154
diff --git a/test/other/codesize/test_codesize_cxx_lto.gzsize b/test/other/codesize/test_codesize_cxx_lto.gzsize
@@ -1 +1 @@
-8346
+8350
diff --git a/test/other/codesize/test_codesize_cxx_lto.jssize b/test/other/codesize/test_codesize_cxx_lto.jssize
@@ -1 +1 @@
-20331
+20337
diff --git a/test/other/codesize/test_codesize_cxx_mangle.gzsize b/test/other/codesize/test_codesize_cxx_mangle.gzsize
@@ -1 +1 @@
-9341
+9343
diff --git a/test/other/codesize/test_codesize_cxx_mangle.jssize b/test/other/codesize/test_codesize_cxx_mangle.jssize
@@ -1 +1 @@
-24023
+24029
diff --git a/test/other/codesize/test_codesize_cxx_noexcept.gzsize b/test/other/codesize/test_codesize_cxx_noexcept.gzsize
@@ -1 +1 @@
-8332
+8335
diff --git a/test/other/codesize/test_codesize_cxx_noexcept.jssize b/test/other/codesize/test_codesize_cxx_noexcept.jssize
@@ -1 +1 @@
-20255
+20261
diff --git a/test/other/codesize/test_codesize_files_js_fs.gzsize b/test/other/codesize/test_codesize_files_js_fs.gzsize
@@ -1 +1 @@
-7640
+7643
diff --git a/test/other/codesize/test_codesize_files_js_fs.jssize b/test/other/codesize/test_codesize_files_js_fs.jssize
@@ -1 +1 @@
-18801
+18807
diff --git a/test/test_core.py b/test/test_core.py
@@ -5572,7 +5572,7 @@ def test_libgen(self):
   def test_utime(self):
     self.do_runf('utime/test_utime.c', 'success')
 
-  @also_with_noderawfs
+  @also_with_nodefs_both
   def test_futimens(self):
     self.do_runf('utime/test_futimens.c', 'success')
 

diff --git a/test/utime/test_futimens.c b/test/utime/test_futimens.c
@@ -14,6 +14,7 @@
 #include <unistd.h>
 #include <time.h>
 #include <sys/stat.h>
+#include <utime.h>
 
 
 void create_file(const char *path, const char *buffer, int mode) {
@@ -67,7 +68,7 @@ void test() {
   assert(s.st_rdev == 0);
   assert(s.st_size == 8);
   assert(s.st_ctime);
-#if defined(__EMSCRIPTEN__) && !defined(NODERAWFS)
+#if defined(__EMSCRIPTEN__) && !defined(NODERAWFS) && !defined(NODEFS)
   assert(s.st_blksize == 4096);
   assert(s.st_blocks == 1);
 #endif
@@ -128,6 +129,15 @@ void test() {
   times[1].tv_nsec = now.tv_nsec;
   check_times(fd, times, 1);
 
+  printf("check setting time to 0...\n");
+  struct utimbuf tb = {0};
+  utime("folder/file", &tb);
+  times[0].tv_sec = 0;
+  times[0].tv_nsec = 0;
+  times[1].tv_sec = 0;
+  times[1].tv_nsec = 0;
+  check_times(fd, times, 0);
+
   close(fd);
 
   puts("success");