Rationalize File timestamp callback

cores/esp8266/FS.cpp

@@ -206,6 +206,7 @@ void File::setTimeCallback(time_t (*cb)(void)) {
     if (!_p)
         return;
     _p->setTimeCallback(cb);
+    _timeCallback = cb;
 }
 
 File Dir::openFile(const char* mode) {
@@ -221,7 +222,7 @@ File Dir::openFile(const char* mode) {
     }
 
     File f(_impl->openFile(om, am), _baseFS);
-    f.setTimeCallback(timeCallback);
+    f.setTimeCallback(_timeCallback);
     return f;
 }
 
@@ -287,7 +288,7 @@ void Dir::setTimeCallback(time_t (*cb)(void)) {
     if (!_impl)
         return;
     _impl->setTimeCallback(cb);
-    timeCallback = cb;
+    _timeCallback = cb;
 }
 
 
@@ -304,7 +305,7 @@ bool FS::begin() {
         DEBUGV("#error: FS: no implementation");
         return false;
     }
-    _impl->setTimeCallback(timeCallback);
+    _impl->setTimeCallback(_timeCallback);
     bool ret = _impl->begin();
     DEBUGV("%s\n", ret? "": "#error: FS could not start");
     return ret;
@@ -367,7 +368,7 @@ File FS::open(const char* path, const char* mode) {
         return File();
     }
     File f(_impl->open(path, om, am), this);
-    f.setTimeCallback(timeCallback);
+    f.setTimeCallback(_timeCallback);
     return f;
 }
 
@@ -388,7 +389,7 @@ Dir FS::openDir(const char* path) {
     }
     DirImplPtr p = _impl->openDir(path);
     Dir d(p, this);
-    d.setTimeCallback(timeCallback);
+    d.setTimeCallback(_timeCallback);
     return d;
 }
 
@@ -444,6 +445,7 @@ void FS::setTimeCallback(time_t (*cb)(void)) {
     if (!_impl)
         return;
     _impl->setTimeCallback(cb);
+    _timeCallback = cb;
 }
 
 

cores/esp8266/FS.h

@@ -118,6 +118,7 @@ class File : public Stream
 
 protected:
     FileImplPtr _p;
+    time_t (*_timeCallback)(void) = nullptr;
 
     // Arduino SD class emulation
     std::shared_ptr<Dir> _fakeDir;
@@ -145,7 +146,7 @@ class Dir {
 protected:
     DirImplPtr _impl;
     FS       *_baseFS;
-    time_t (*timeCallback)(void) = nullptr;
+    time_t (*_timeCallback)(void) = nullptr;
 };
 
 // Backwards compatible, <4GB filesystem usage
@@ -198,7 +199,7 @@ class SPIFFSConfig : public FSConfig
 class FS
 {
 public:
-    FS(FSImplPtr impl) : _impl(impl) { timeCallback = _defaultTimeCB; }
+    FS(FSImplPtr impl) : _impl(impl) { _timeCallback = _defaultTimeCB; }
 
     bool setConfig(const FSConfig &cfg);
 
@@ -240,7 +241,7 @@ class FS
 protected:
     FSImplPtr _impl;
     FSImplPtr getImpl() { return _impl; }
-    time_t (*timeCallback)(void);
+    time_t (*_timeCallback)(void) = nullptr;
     static time_t _defaultTimeCB(void) { return time(NULL); }
 };
 

cores/esp8266/FSImpl.h

@@ -45,8 +45,8 @@ class FileImpl {
 
     // Filesystems *may* support a timestamp per-file, so allow the user to override with
     // their own callback for *this specific* file (as opposed to the FSImpl call of the
-    // same name.  The default implementation simply returns time(&null)
-    virtual void setTimeCallback(time_t (*cb)(void)) { timeCallback = cb; }
+    // same name.  The default implementation simply returns time(null)
+    virtual void setTimeCallback(time_t (*cb)(void)) { _timeCallback = cb; }
 
     // Return the last written time for a file.  Undefined when called on a writable file
     // as the FS is allowed to return either the time of the last write() operation or the
@@ -56,7 +56,7 @@ class FileImpl {
     virtual time_t getCreationTime() { return 0; } // Default is to not support timestamps
 
 protected:
-    time_t (*timeCallback)(void) = nullptr;
+    time_t (*_timeCallback)(void) = nullptr;
 };
 
 enum OpenMode {
@@ -90,11 +90,11 @@ class DirImpl {
 
     // Filesystems *may* support a timestamp per-file, so allow the user to override with
     // their own callback for *this specific* file (as opposed to the FSImpl call of the
-    // same name.  The default implementation simply returns time(&null)
-    virtual void setTimeCallback(time_t (*cb)(void)) { timeCallback = cb; }
+    // same name.  The default implementation simply returns time(null)
+    virtual void setTimeCallback(time_t (*cb)(void)) { _timeCallback = cb; }
 
 protected:
-    time_t (*timeCallback)(void) = nullptr;
+    time_t (*_timeCallback)(void) = nullptr;
 };
 
 class FSImpl {
@@ -118,11 +118,11 @@ class FSImpl {
 
     // Filesystems *may* support a timestamp per-file, so allow the user to override with
     // their own callback for all files on this FS.  The default implementation simply
-    // returns the present time as reported by time(&null)
-    virtual void setTimeCallback(time_t (*cb)(void)) { timeCallback = cb; }
+    // returns the present time as reported by time(null)
+    virtual void setTimeCallback(time_t (*cb)(void)) { _timeCallback = cb; }
 
 protected:
-    time_t (*timeCallback)(void) = nullptr;
+    time_t (*_timeCallback)(void) = nullptr;
 };
 
 } // namespace fs

libraries/LittleFS/src/LittleFS.cpp

@@ -70,14 +70,14 @@ FileImplPtr LittleFSImpl::open(const char* path, OpenMode openMode, AccessMode a
     }
 
     time_t creation = 0;
-    if (timeCallback && (openMode & OM_CREATE)) {
+    if (_timeCallback && (openMode & OM_CREATE)) {
         // O_CREATE means we *may* make the file, but not if it already exists.
         // See if it exists, and only if not update the creation time
         int rc = lfs_file_open(&_lfs, fd.get(), path, LFS_O_RDONLY);
 	if (rc == 0) {
             lfs_file_close(&_lfs, fd.get()); // It exists, don't update create time
         } else {
-            creation = timeCallback();  // File didn't exist or otherwise, so we're going to create this time
+            creation = _timeCallback();  // File didn't exist or otherwise, so we're going to create this time
         }
     }
 

libraries/LittleFS/src/LittleFS.h

@@ -424,7 +424,7 @@ class LittleFSFileImpl : public FileImpl
             lfs_file_close(_fs->getFS(), _getFD());
             _opened = false;
             DEBUGV("lfs_file_close: fd=%p\n", _getFD());
-            if (timeCallback && (_flags & LFS_O_WRONLY)) {
+            if (_timeCallback && (_flags & LFS_O_WRONLY)) {
                 // If the file opened with O_CREAT, write the creation time attribute
                 if (_creation) {
                     int rc = lfs_setattr(_fs->getFS(), _name.get(), 'c', (const void *)&_creation, sizeof(_creation));
@@ -433,7 +433,7 @@ class LittleFSFileImpl : public FileImpl
                     }
                 }
                 // Add metadata with last write time
-                time_t now = timeCallback();
+                time_t now = _timeCallback();
                 int rc = lfs_setattr(_fs->getFS(), _name.get(), 't', (const void *)&now, sizeof(now));
                 if (rc < 0) {
                     DEBUGV("Unable to set last write time on '%s' to %d\n", _name.get(), now);

tests/host/fs/test_fs.cpp

@@ -162,35 +162,4 @@ TEST_CASE("SD.h FILE_WRITE macro is append", "[fs]")
     REQUIRE(u == 0);
 }
 
-// SDFS timestamp setter (#7682)
-static time_t _my_time(void)
-{
-    struct tm t;
-    bzero(&t, sizeof(t));
-    t.tm_year = 120;
-    t.tm_mon  = 9;
-    t.tm_mday = 22;
-    t.tm_hour = 12;
-    t.tm_min  = 13;
-    t.tm_sec  = 14;
-    return mktime(&t);
-}
-
-TEST_CASE("SDFS timeCallback")
-{
-    SDFS_MOCK_DECLARE(64, 8, 512, "");
-    REQUIRE(SDFS.begin());
-    REQUIRE(SD.begin(4));
-
-    SDFS.setTimeCallback(_my_time);
-    File f = SD.open("/file.txt", "w");
-    f.write("Had we but world enough, and time,");
-    f.close();
-    time_t expected = _my_time();
-    f = SD.open("/file.txt", "r");
-    REQUIRE(abs(f.getCreationTime() - expected) < 60);  // FAT has less precision in timestamp than time_t
-    REQUIRE(abs(f.getLastWrite() - expected) < 60);  // FAT has less precision in timestamp than time_t
-    f.close();
-}
-
 };

tests/host/fs/test_fs.inc

@@ -220,6 +220,40 @@ TEST_CASE(TESTPRE "Rewriting file frees space immediately (#7426)", TESTPAT)
     }
 }
 
+#if FSTYPE != SPIFFS
+
+// Timestamp setter (#7682, #7775)
+static time_t _my_time(void)
+{
+    struct tm t;
+    bzero(&t, sizeof(t));
+    t.tm_year = 120;
+    t.tm_mon  = 9;
+    t.tm_mday = 22;
+    t.tm_hour = 12;
+    t.tm_min  = 13;
+    t.tm_sec  = 14;
+    return mktime(&t);
+}
+
+TEST_CASE("Verify timeCallback works properly")
+{
+    FS_MOCK_DECLARE(64, 8, 512, "");
+    REQUIRE(FSTYPE.begin());
+
+    FSTYPE.setTimeCallback(_my_time);
+    File f = FSTYPE.open("/file.txt", "w");
+    f.write("Had we but world enough, and time,");
+    f.close();
+    time_t expected = _my_time();
+    f = FSTYPE.open("/file.txt", "r");
+    REQUIRE(abs(f.getCreationTime() - expected) < 60);  // FAT has less precision in timestamp than time_t
+    REQUIRE(abs(f.getLastWrite() - expected) < 60);  // FAT has less precision in timestamp than time_t
+    f.close();
+}
+
+#endif
+
 #ifdef FS_HAS_DIRS
 
 #if FSTYPE != SDFS