pw_persistent_ram: Make persistents mutable

Adds an accessor to pw_persistent that allows non-const access to the
underlying object to enable in-place modification.

Change-Id: Ibcc73ee66cf90c2904a3f7d5380b6c7c3a372f3c
Reviewed-on: https://pigweed-review.googlesource.com/c/pigweed/pigweed/+/38721
Pigweed-Auto-Submit: Armando Montanez <amontanez@google.com>
Reviewed-by: Ewout van Bekkum <ewout@google.com>
Reviewed-by: Wyatt Hepler <hepler@google.com>
Commit-Queue: Auto-Submit <auto-submit@pigweed.google.com.iam.gserviceaccount.com>
diff --git a/pw_persistent_ram/docs.rst b/pw_persistent_ram/docs.rst
index e307b99..75e8175 100644
--- a/pw_persistent_ram/docs.rst
+++ b/pw_persistent_ram/docs.rst
@@ -117,8 +117,8 @@
 The destructor does nothing, ergo it is okay if it is not executed during
 shutdown.
 
-Example
--------
+Example: Storing an integer
+---------------------------
 A common use case of persistent data is to track boot counts, or effectively
 how often the device has rebooted. This can be useful for monitoring how many
 times the device rebooted and/or crashed. This can be easily accomplished using
@@ -158,6 +158,57 @@
       // ... rest of main
     }
 
+Example: Storing larger objects
+-------------------------------
+Larger objects may be inefficient to copy back and forth due to the need for
+a working copy. To work around this, you can get a Mutator handle that provides
+direct access to the underlying object. As long as the Mutator is in scope, it
+is invalid to access the underlying Persistent, but you'll be able to directly
+modify the object in place. Once the Mutator goes out of scope, the Persistent
+object's checksum is updated to reflect the changes.
+
+.. code-block:: cpp
+
+    #include "pw_persistent_ram/persistent.h"
+    #include "pw_preprocessor/compiler.h"
+
+    using pw::persistent_ram::Persistent;
+
+    contexpr size_t kMaxReasonLength = 256;
+
+    struct LastCrashInfo {
+      uint32_t uptime_ms;
+      uint32_t boot_id;
+      char reason[kMaxReasonLength];
+    }
+
+    PW_KEEP_IN_SECTION(".noinit") Persistent<LastBootInfo> persistent_crash_info;
+
+    void HandleCrash(const char* fmt, va_list args) {
+      // Once this scope ends, we know the persistent object has been updated
+      // to reflect changes.
+      {
+        auto& mutable_crash_info = persistent_crash_info.mutator();
+        vsnprintf(mutable_crash_info.reason,
+                  sizeof(mutable_crash_info.reason),
+                  fmt,
+                  args);
+        mutable_crash_info.uptime_ms = system::GetUptimeMs();
+        mutable_crash_info.boot_id = system::GetBootId();
+      }
+      // ...
+    }
+
+    int main() {
+      if (persistent_crash_info.has_value()) {
+        LogLastCrashInfo(persistent_crash_info.value());
+        // Clear crash info once it has been dumped.
+        persistent_crash_info.reset();
+      }
+
+      // ... rest of main
+    }
+
 Size Report
 -----------
 The following size report showcases the overhead for using Persistent. Note that
diff --git a/pw_persistent_ram/persistent_test.cc b/pw_persistent_ram/persistent_test.cc
index 6f45489..868ec1d 100644
--- a/pw_persistent_ram/persistent_test.cc
+++ b/pw_persistent_ram/persistent_test.cc
@@ -81,5 +81,58 @@
   EXPECT_EQ(42u, persistent.value());
 }
 
+class MutablePersistentTest : public ::testing::Test {
+ protected:
+  struct Coordinate {
+    int x;
+    int y;
+    int z;
+  };
+  MutablePersistentTest() { ZeroPersistentMemory(); }
+
+  // Emulate invalidation of persistent section(s).
+  void ZeroPersistentMemory() { memset(&buffer_, 0, sizeof(buffer_)); }
+
+  // Allocate a chunk of aligned storage that can be independently controlled.
+  std::aligned_storage_t<sizeof(Persistent<Coordinate>),
+                         alignof(Persistent<Coordinate>)>
+      buffer_;
+};
+
+TEST_F(MutablePersistentTest, DefaultConstructionAndDestruction) {
+  {
+    // Emulate a boot where the persistent sections were invalidated.
+    // Although the fixture always does this, we do this an extra time to be
+    // 100% confident that an integrity check cannot be accidentally selected
+    // which results in reporting there is valid data when zero'd.
+    ZeroPersistentMemory();
+    auto& persistent = *(new (&buffer_) Persistent<Coordinate>());
+    EXPECT_FALSE(persistent.has_value());
+
+    // Default construct of a Coordinate.
+    persistent.emplace(Coordinate({.x = 5, .y = 6, .z = 7}));
+    ASSERT_TRUE(persistent.has_value());
+    {
+      auto mutable_persistent = persistent.mutator();
+      mutable_persistent->x = 42;
+      (*mutable_persistent).y = 1337;
+      mutable_persistent->z = -99;
+      ASSERT_FALSE(persistent.has_value());
+    }
+
+    EXPECT_EQ(1337, persistent.value().y);
+    EXPECT_EQ(-99, persistent.value().z);
+
+    persistent.~Persistent();  // Emulate shutdown / global destructors.
+  }
+
+  {
+    // Emulate a boot where persistent memory was kept as is.
+    auto& persistent = *(new (&buffer_) Persistent<Coordinate>());
+    ASSERT_TRUE(persistent.has_value());
+    EXPECT_EQ(42, persistent.value().x);
+  }
+}
+
 }  // namespace
 }  // namespace pw::persistent_ram
diff --git a/pw_persistent_ram/public/pw_persistent_ram/persistent.h b/pw_persistent_ram/public/pw_persistent_ram/persistent.h
index 06f356b..9d45465 100644
--- a/pw_persistent_ram/public/pw_persistent_ram/persistent.h
+++ b/pw_persistent_ram/public/pw_persistent_ram/persistent.h
@@ -46,6 +46,37 @@
 template <typename T>
 class Persistent {
  public:
+  // This object provides mutable access to the underlying object of a
+  // Persistent<T>.
+  //
+  // WARNING: This object must remain in scope for any modifications of the
+  // Underlying object. If the object is modified after the Mutator goes out
+  // of scope, the CRC will not be updated to reflect changes, invalidating the
+  // contents of the Persistent<T>!
+  //
+  // WARNING: Persistent<T>::has_value() will return false if there are
+  // in-flight modifications by a Mutator that have not yet been flushed.
+  class Mutator {
+   public:
+    explicit constexpr Mutator(Persistent<T>& persistent)
+        : persistent_(persistent) {}
+    ~Mutator() { persistent_.crc_ = persistent_.CalculateCrc(); }
+
+    Mutator(const Mutator&) = delete;  // Copy constructor is disabled.
+
+    T* operator->() { return const_cast<T*>(&persistent_.contents_); }
+
+    // Be careful when sharing a reference or pointer to the underlying object.
+    // Once the Mutator goes out of scope, any changes to the object will
+    // invalidate the checksum. Avoid directly using the underlying object
+    // unless you need to pass it to a function.
+    T& value() { return persistent_.contents_; }
+    T& operator*() { return *const_cast<T*>(&persistent_.contents_); }
+
+   private:
+    Persistent<T>& persistent_;
+  };
+
   // Constructor which does nothing, meaning it never sets the value.
   constexpr Persistent() {}
 
@@ -57,7 +88,7 @@
   template <class... Args>
   const T& emplace(Args&&... args) {
     new (const_cast<T*>(&contents_)) T(std::forward<Args>(args)...);
-    crc_ = Crc(contents_);
+    crc_ = CalculateCrc();
     return const_cast<T&>(contents_);
   }
 
@@ -65,7 +96,7 @@
   template <typename U = T>
   Persistent& operator=(U&& value) {
     contents_ = std::move(value);
-    crc_ = Crc(contents_);
+    crc_ = CalculateCrc();
     return *this;
   }
 
@@ -78,7 +109,7 @@
 
   // Returns true if a value is held by the Persistent.
   bool has_value() const {
-    return crc_ == Crc(contents_);  // There's a value if its CRC matches.
+    return crc_ == CalculateCrc();  // There's a value if its CRC matches.
   }
 
   // Access the value.
@@ -89,18 +120,29 @@
     return const_cast<T&>(contents_);
   }
 
+  // Get a mutable handle to the underlying data.
+  //
+  // Precondition: has_value() must be true.
+  Mutator mutator() {
+    PW_ASSERT(has_value());
+    return Mutator(*this);
+  }
+
  private:
+  friend class Mutator;
+
   static_assert(std::is_trivially_copy_constructible<T>::value,
                 "If a Persistent persists across reboots, it is effectively "
                 "loaded through a trivial copy constructor.");
+
   static_assert(std::is_trivially_destructible<T>::value,
                 "A Persistent's destructor does not invoke the value's "
                 "destructor, ergo only trivially destructible types are "
                 "supported.");
 
-  static uint16_t Crc(volatile const T& value) {
+  uint16_t CalculateCrc() const {
     return checksum::Crc16Ccitt::Calculate(
-        std::as_bytes(std::span(const_cast<const T*>(&value), 1)));
+        std::as_bytes(std::span(const_cast<const T*>(&contents_), 1)));
   }
 
   // Use unions to denote that these members are never initialized by design and