Write config files through a temp file and rename (nRF52, STM32)

examples/companion_radio/DataStore.cpp

@@ -1,5 +1,6 @@
 #include <Arduino.h>
 #include "DataStore.h"
+#include <helpers/AtomicFile.h>
 
 #if defined(EXTRAFS) || defined(QSPIFLASH)
   #define MAX_BLOBRECS 100
@@ -31,15 +32,10 @@ DataStore::DataStore(FILESYSTEM& fs, FILESYSTEM& fsExtra, mesh::RTCClock& clock)
 }
 #endif
 
+// Plain remove-and-rewrite, for the copy/migration code, the in-place adv_blobs
+// file and the ESP blob files. Config files go through AtomicFile::write().
 static File openWrite(FILESYSTEM* fs, const char* filename) {
-#if defined(NRF52_PLATFORM) || defined(STM32_PLATFORM)
-  fs->remove(filename);
-  return fs->open(filename, FILE_O_WRITE);
-#elif defined(RP2040_PLATFORM)
-  return fs->open(filename, "w");
-#else
-  return fs->open(filename, "w", true);
-#endif
+  return AtomicFile::openPlain(fs, filename);
 }
 
 #if defined(NRF52_PLATFORM) || defined(STM32_PLATFORM)
@@ -190,6 +186,7 @@ bool DataStore::saveMainIdentity(const mesh::LocalIdentity &identity) {
 }
 
 void DataStore::loadPrefs(NodePrefs& prefs) {
+  AtomicFile::cleanup(_fs, "/prefs.json");
   if (_fs->exists("/prefs.json")) {
     File file = openRead(_fs, "/prefs.json");
     if (file) {
@@ -247,16 +244,13 @@ void DataStore::loadPrefsInt(const char *filename, NodePrefs& _prefs) {
 }
 
 bool DataStore::savePrefs(NodePrefs& _prefs) {
-  File file = openWrite(_fs, "/prefs.json");
-  if (file) {
-    bool success = _prefs.saveSerial(file);
-    file.close();
-    return success;
-  }
-  return false;
+  return AtomicFile::write(_fs, "/prefs.json", [&](File& file) {
+    return _prefs.saveSerial(file);
+  });
 }
 
 void DataStore::loadContacts(DataStoreHost* host) {
+AtomicFile::cleanup(_getContactsChannelsFS(), "/contacts3");
 File file = openRead(_getContactsChannelsFS(), "/contacts3");
     if (file) {
       bool full = false;
@@ -288,8 +282,7 @@ File file = openRead(_getContactsChannelsFS(), "/contacts3");
 }
 
 void DataStore::saveContacts(DataStoreHost* host, bool (*filter)(const ContactInfo& c)) {
-  File file = openWrite(_getContactsChannelsFS(), "/contacts3");
-  if (file) {
+  AtomicFile::write(_getContactsChannelsFS(), "/contacts3", [&](File& file) {
     uint32_t idx = 0;
     ContactInfo c;
     uint8_t unused = 0;
@@ -312,15 +305,16 @@ void DataStore::saveContacts(DataStoreHost* host, bool (*filter)(const ContactIn
       success = success && (file.write((uint8_t *)&c.gps_lat, 4) == 4);
       success = success && (file.write((uint8_t *)&c.gps_lon, 4) == 4);
 
-      if (!success) break; // write failed
+      if (!success) return false; // write failed
 
       idx++;  // advance to next contact
     }
-    file.close();
-  }
+    return true;
+  });
 }
 
 void DataStore::loadChannels(DataStoreHost* host) {
+    AtomicFile::cleanup(_getContactsChannelsFS(), "/channels2");
     File file = openRead(_getContactsChannelsFS(), "/channels2");
     if (file) {
       bool full = false;
@@ -346,8 +340,7 @@ void DataStore::loadChannels(DataStoreHost* host) {
 }
 
 void DataStore::saveChannels(DataStoreHost* host) {
-  File file = openWrite(_getContactsChannelsFS(), "/channels2");
-  if (file) {
+  AtomicFile::write(_getContactsChannelsFS(), "/channels2", [&](File& file) {
     uint8_t channel_idx = 0;
     ChannelDetails ch;
     uint8_t unused[4];
@@ -358,11 +351,11 @@ void DataStore::saveChannels(DataStoreHost* host) {
       success = success && (file.write((uint8_t *)ch.name, 32) == 32);
       success = success && (file.write((uint8_t *)ch.channel.secret, 32) == 32);
 
-      if (!success) break; // write failed
+      if (!success) return false; // write failed
       channel_idx++;
     }
-    file.close();
-  }
+    return true;
+  });
 }
 
 #if defined(NRF52_PLATFORM) || defined(STM32_PLATFORM)

platformio.ini

@@ -166,7 +166,9 @@ build_flags = -std=c++17
   -I src
   -I test/mocks
 test_build_src = yes
-test_ignore = test_kiss_modem
+test_ignore =
+  test_kiss_modem
+  test_atomic_file
 build_src_filter =
   -<*>
   +<../src/Utils.cpp>
@@ -176,6 +178,21 @@ build_src_filter =
 lib_deps =
   google/googletest @ 1.17.0
 
+[env:native_atomic_file]
+platform = native
+test_framework = googletest
+build_flags = -std=c++17
+  -I test/mocks
+  -I src
+  -D NRF52_PLATFORM
+test_build_src = yes
+test_filter = test_atomic_file
+build_src_filter =
+  -<*>
+  +<../src/helpers/AtomicFile.cpp>
+lib_deps =
+  google/googletest @ 1.17.0
+
 [env:native_kiss_modem]
 platform = native
 test_framework = googletest

src/helpers/AtomicFile.cpp

@@ -0,0 +1,37 @@
+#include "AtomicFile.h"
+
+#ifdef MESH_ATOMIC_FS_WRITES
+
+#include <string.h>
+
+namespace AtomicFile {
+
+bool tmpName(const char* path, char* dest, size_t dest_size) {
+  size_t n = strlen(path);
+  if (n + 5 > dest_size) return false;
+  memcpy(dest, path, n);
+  memcpy(dest + n, ".tmp", 5);
+  return true;
+}
+
+void cleanup(FILESYSTEM* fs, const char* path) {
+  char tmp[48];
+  if (tmpName(path, tmp, sizeof(tmp)) && fs->exists(tmp)) {
+    fs->remove(tmp);
+  }
+}
+
+bool publish(FILESYSTEM* fs, const char* tmp_path, const char* path, uint32_t expected_size) {
+  // close() returns void and drops the filesystem's error, so check what
+  // actually reached the medium.
+  File f = fs->open(tmp_path, FILE_O_READ);
+  if (!f) return false;
+  bool same = (f.size() == expected_size);
+  f.close();
+  if (!same) return false;
+  return fs->rename(tmp_path, path);
+}
+
+}
+
+#endif

src/helpers/AtomicFile.h

@@ -0,0 +1,90 @@
+#pragma once
+
+// Crash-safe replacement for the "remove(file), then open and write" pattern.
+//
+// On littlefs platforms (nRF52, STM32) the new content is written to
+// "<path>.tmp", closed, read back for a size check and only then renamed over
+// <path>. lfs_rename() replaces an existing target in one commit, so a power
+// loss leaves either the old or the new file, never a missing or short one.
+// The old file is never removed before the new one is complete.
+//
+// The temp copy needs free space for a second copy of the file. If it does not
+// fit (or the write or rename fails), write() falls back to the previous
+// behaviour: remove the old file and write <path> in place.
+//
+// Other platforms (ESP32, RP2040) keep the previous behaviour unchanged. Define
+// MESH_ATOMIC_FS_WRITES to switch the atomic path on there once rename() over
+// an existing file is known to work on that filesystem.
+
+#include <stddef.h>
+#include <stdint.h>
+
+#if defined(ESP32) || defined(RP2040_PLATFORM)
+  #include <FS.h>
+  #ifndef FILESYSTEM
+    #define FILESYSTEM  fs::FS
+  #endif
+#elif defined(NRF52_PLATFORM) || defined(STM32_PLATFORM)
+  #include <Adafruit_LittleFS.h>
+  #ifndef FILESYSTEM
+    #define FILESYSTEM  Adafruit_LittleFS
+  #endif
+  using namespace Adafruit_LittleFS_Namespace;
+#endif
+
+#if (defined(NRF52_PLATFORM) || defined(STM32_PLATFORM)) && !defined(MESH_ATOMIC_FS_WRITES)
+  #define MESH_ATOMIC_FS_WRITES 1
+#endif
+
+namespace AtomicFile {
+
+#ifdef MESH_ATOMIC_FS_WRITES
+// Removes a "<path>.tmp" left behind by an interrupted write(). The file at
+// <path> is never half-written, so an orphaned temp copy is always an
+// unfinished one and is never adopted. Call before loading <path>.
+void cleanup(FILESYSTEM* fs, const char* path);
+// Size-checks the closed temp file and renames it over <path>.
+bool publish(FILESYSTEM* fs, const char* tmp_path, const char* path, uint32_t expected_size);
+bool tmpName(const char* path, char* dest, size_t dest_size);
+#else
+inline void cleanup(FILESYSTEM*, const char*) { }
+#endif
+
+inline File openPlain(FILESYSTEM* fs, const char* path) {
+#if defined(NRF52_PLATFORM) || defined(STM32_PLATFORM)
+  fs->remove(path);
+  return fs->open(path, FILE_O_WRITE);
+#elif defined(RP2040_PLATFORM)
+  return fs->open(path, "w");
+#else
+  return fs->open(path, "w", true);
+#endif
+}
+
+// Runs body(File&) against a fresh file and makes it <path> when body() returns
+// true. body() may be called a second time (fallback), so it must start from
+// scratch each time. Returns body()'s result, false if nothing could be saved.
+template <class Body>
+bool write(FILESYSTEM* fs, const char* path, Body body) {
+#ifdef MESH_ATOMIC_FS_WRITES
+  char tmp[48];
+  if (tmpName(path, tmp, sizeof(tmp))) {
+    fs->remove(tmp);
+    File f = fs->open(tmp, FILE_O_WRITE);
+    if (f) {
+      bool ok = body(f);
+      uint32_t written = f.position();
+      f.close();
+      if (ok && publish(fs, tmp, path, written)) return true;
+    }
+    fs->remove(tmp);   // old file is untouched, now try the in-place way
+  }
+#endif
+  File f = openPlain(fs, path);
+  if (!f) return false;
+  bool ok = body(f);
+  f.close();
+  return ok;
+}
+
+}

src/helpers/ClientACL.cpp

@@ -1,19 +1,10 @@
 #include "ClientACL.h"
-
-static File openWrite(FILESYSTEM* _fs, const char* filename) {
-  #if defined(NRF52_PLATFORM) || defined(STM32_PLATFORM)
-    _fs->remove(filename);
-    return _fs->open(filename, FILE_O_WRITE);
-  #elif defined(RP2040_PLATFORM)
-    return _fs->open(filename, "w");
-  #else
-    return _fs->open(filename, "w", true);
-  #endif
-}
+#include "AtomicFile.h"
 
 void ClientACL::load(FILESYSTEM* fs, const mesh::LocalIdentity& self_id) {
   _fs = fs;
   num_clients = 0;
+  AtomicFile::cleanup(_fs, "/s_contacts");
   if (_fs->exists("/s_contacts")) {
   #if defined(RP2040_PLATFORM)
     File file = _fs->open("/s_contacts", "r");
@@ -54,8 +45,7 @@ void ClientACL::load(FILESYSTEM* fs, const mesh::LocalIdentity& self_id) {
 
 void ClientACL::save(FILESYSTEM* fs, bool (*filter)(ClientInfo*)) {
   _fs = fs;
-  File file = openWrite(_fs, "/s_contacts");
-  if (file) {
+  AtomicFile::write(_fs, "/s_contacts", [&](File& file) {
     uint8_t unused[2];
     memset(unused, 0, sizeof(unused));
 
@@ -71,10 +61,10 @@ void ClientACL::save(FILESYSTEM* fs, bool (*filter)(ClientInfo*)) {
       success = success && (file.write(c->out_path, 64) == 64);
       success = success && (file.write(c->shared_secret, PUB_KEY_SIZE) == PUB_KEY_SIZE);
 
-      if (!success) break; // write failed
+      if (!success) return false; // write failed
     }
-    file.close();
-  }
+    return true;
+  });
 }
 
 bool ClientACL::clear() {

src/helpers/CommonCLI.cpp

@@ -1,5 +1,6 @@
 #include <Arduino.h>
 #include "CommonCLI.h"
+#include "AtomicFile.h"
 #include "TxtDataHelpers.h"
 #include "AdvertDataHelpers.h"
 #include "TxtDataHelpers.h"
@@ -28,6 +29,7 @@ static bool isValidName(const char *n) {
 }
 
 void CommonCLI::loadPrefs(FILESYSTEM* fs) {
+  AtomicFile::cleanup(fs, "/prefs.json");
   if (fs->exists("/prefs.json")) {
 #if defined(RP2040_PLATFORM)
     File file = fs->open("/prefs.json", "r");
@@ -141,20 +143,9 @@ void CommonCLI::loadPrefsInt(FILESYSTEM* fs, const char* filename) {  // Legacy
 }
 
 bool CommonCLI::savePrefs(FILESYSTEM* fs) {
-#if defined(NRF52_PLATFORM) || defined(STM32_PLATFORM)
-  fs->remove("/prefs.json");
-  File file = fs->open("/prefs.json", FILE_O_WRITE);
-#elif defined(RP2040_PLATFORM)
-  File file = fs->open("/prefs.json", "w");
-#else
-  File file = fs->open("/prefs.json", "w", true);
-#endif
-  if (file) {
-    bool success = _prefs->saveSerial(file);
-    file.close();
-    return success;
-  }
-  return false;
+  return AtomicFile::write(fs, "/prefs.json", [&](File& file) {
+    return _prefs->saveSerial(file);
+  });
 }
 
 #define MIN_LOCAL_ADVERT_INTERVAL   60