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