pw_kvs: Move EntryHeaderFormat to pw_kvs/format.h

- Move EntryHeaderFormat and the EntryHeader struct to pw_kvs/format.h.
- Rename EntryHeaderFormat to EntryFormat.

Change-Id: Ic5e53c688cd3a483101c7985def6e4bef9c3cbaf

12 files changed, 119 insertions, 95 deletions

diff --git a/pw_kvs/BUILD b/pw_kvs/BUILD
index 64c44b302..0bb21205b 100644
--- a/pw_kvs/BUILD
+++ b/pw_kvs/BUILD
@@ -41,6 +41,7 @@ pw_cc_library(
         "public/pw_kvs/checksum.h",
         "public/pw_kvs/crc16_checksum.h",
         "public/pw_kvs/flash_memory.h",
+        "public/pw_kvs/format.h",
         "public/pw_kvs/key_value_store.h",
         "public/pw_kvs/output.h",
     ],
diff --git a/pw_kvs/BUILD.gn b/pw_kvs/BUILD.gn
index 06bf49727..237ec7f02 100644
--- a/pw_kvs/BUILD.gn
+++ b/pw_kvs/BUILD.gn
@@ -25,6 +25,7 @@ source_set("pw_kvs") {
     "public/pw_kvs/alignment.h",
     "public/pw_kvs/checksum.h",
     "public/pw_kvs/flash_memory.h",
+    "public/pw_kvs/format.h",
     "public/pw_kvs/key_value_store.h",
     "public/pw_kvs/output.h",
   ]
diff --git a/pw_kvs/debug_cli.cc b/pw_kvs/debug_cli.cc
index 7d2c744a1..11519adc6 100644
--- a/pw_kvs/debug_cli.cc
+++ b/pw_kvs/debug_cli.cc
@@ -26,7 +26,7 @@ namespace {
 using std::byte;
 
 ChecksumCrc16 checksum;
-constexpr EntryHeaderFormat format{.magic = 0xBAD'C0D3, .checksum = &checksum};
+constexpr EntryFormat format{.magic = 0xBAD'C0D3, .checksum = &checksum};
 
 constexpr char kHelpText[] = R"(
 pw_kvs debug CLI
diff --git a/pw_kvs/entry.cc b/pw_kvs/entry.cc
index c8e62626d..086c014be 100644
--- a/pw_kvs/entry.cc
+++ b/pw_kvs/entry.cc
@@ -54,8 +54,7 @@ Status Entry::ReadKey(FlashPartition& partition,
 
 Entry::Entry(FlashPartition& partition,
              Address address,
-             uint32_t magic,
-             ChecksumAlgorithm* algorithm,
+             const EntryFormat& format,
              string_view key,
              span<const byte> value,
              uint16_t value_size_bytes,
@@ -63,14 +62,14 @@ Entry::Entry(FlashPartition& partition,
              uint32_t transaction_id)
     : Entry(&partition,
             address,
-            {.magic = magic,
+            {.magic = format.magic,
              .checksum = 0,
              .alignment_units = alignment_bytes_to_units(alignment_bytes),
              .key_length_bytes = static_cast<uint8_t>(key.size()),
              .value_size_bytes = value_size_bytes,
              .transaction_id = transaction_id}) {
-  if (algorithm != nullptr) {
-    span<const byte> checksum = CalculateChecksum(algorithm, key, value);
+  if (format.checksum != nullptr) {
+    span<const byte> checksum = CalculateChecksum(format.checksum, key, value);
     std::memcpy(&header_.checksum,
                 checksum.data(),
                 std::min(checksum.size(), sizeof(header_.checksum)));
diff --git a/pw_kvs/entry_test.cc b/pw_kvs/entry_test.cc
index 99ef7b15d..65c7f640b 100644
--- a/pw_kvs/entry_test.cc
+++ b/pw_kvs/entry_test.cc
@@ -20,6 +20,7 @@
 #include "pw_kvs/alignment.h"
 #include "pw_kvs/crc16_checksum.h"
 #include "pw_kvs/flash_memory.h"
+#include "pw_kvs/format.h"
 #include "pw_kvs/in_memory_fake_flash.h"
 #include "pw_kvs_private/byte_utils.h"
 #include "pw_span/span.h"
@@ -30,6 +31,8 @@ namespace {
 using std::byte;
 using std::string_view;
 
+constexpr EntryFormat kFormat{0xbeef, nullptr};
+
 TEST(Entry, Size_RoundsUpToAlignment) {
   FakeFlashBuffer<64, 2> flash(16);
   FlashPartition partition(&flash, 0, flash.sector_count());
@@ -39,13 +42,13 @@ TEST(Entry, Size_RoundsUpToAlignment) {
 
     for (size_t value : {size_t(0), align - 1, align, align + 1, 2 * align}) {
       Entry entry = Entry::Valid(
-          partition, 0, 9, nullptr, "k", {nullptr, value}, alignment_bytes, 0);
+          partition, 0, kFormat, "k", {nullptr, value}, alignment_bytes, 0);
       ASSERT_EQ(AlignUp(sizeof(EntryHeader) + 1 /* key */ + value, align),
                 entry.size());
     }
 
     Entry entry =
-        Entry::Tombstone(partition, 0, 9, nullptr, "k", alignment_bytes, 0);
+        Entry::Tombstone(partition, 0, kFormat, "k", alignment_bytes, 0);
     ASSERT_EQ(AlignUp(sizeof(EntryHeader) + 1 /* key */, align), entry.size());
   }
 }
@@ -54,11 +57,11 @@ TEST(Entry, Construct_ValidEntry) {
   FakeFlashBuffer<64, 2> flash(16);
   FlashPartition partition(&flash, 0, flash.sector_count());
 
-  auto entry = Entry::Valid(
-      partition, 1, 9, nullptr, "k", as_bytes(span("123")), 1, 9876);
+  auto entry =
+      Entry::Valid(partition, 1, kFormat, "k", as_bytes(span("123")), 1, 9876);
 
   EXPECT_FALSE(entry.deleted());
-  EXPECT_EQ(entry.magic(), 9u);
+  EXPECT_EQ(entry.magic(), kFormat.magic);
   EXPECT_EQ(entry.value_size(), sizeof("123"));
   EXPECT_EQ(entry.transaction_id(), 9876u);
 }
@@ -67,10 +70,10 @@ TEST(Entry, Construct_Tombstone) {
   FakeFlashBuffer<64, 2> flash(16);
   FlashPartition partition(&flash, 0, flash.sector_count());
 
-  auto entry = Entry::Tombstone(partition, 1, 99, nullptr, "key", 1, 123);
+  auto entry = Entry::Tombstone(partition, 1, kFormat, "key", 1, 123);
 
   EXPECT_TRUE(entry.deleted());
-  EXPECT_EQ(entry.magic(), 99u);
+  EXPECT_EQ(entry.magic(), kFormat.magic);
   EXPECT_EQ(entry.value_size(), 0u);
   EXPECT_EQ(entry.transaction_id(), 123u);
 }
@@ -185,9 +188,10 @@ TEST(ValidEntry, Write) {
   FakeFlashBuffer<1024, 4> flash;
   FlashPartition partition(&flash);
   ChecksumCrc16 checksum;
+  const EntryFormat format{0x600DF00Du, &checksum};
 
-  Entry entry = Entry::Valid(
-      partition, 53, 0x600DF00Du, &checksum, "key45", kValue1, 32, 0x96979899u);
+  Entry entry =
+      Entry::Valid(partition, 53, format, "key45", kValue1, 32, 0x96979899u);
 
   auto result = entry.Write("key45", kValue1);
   EXPECT_EQ(Status::OK, result.status());
@@ -254,9 +258,9 @@ TEST(TombstoneEntry, Write) {
   FakeFlashBuffer<1024, 4> flash;
   FlashPartition partition(&flash);
   ChecksumCrc16 checksum;
+  const EntryFormat format{0x600DF00Du, &checksum};
 
-  Entry entry = Entry::Tombstone(
-      partition, 16, 0x600DF00Du, &checksum, "K", 16, 0x03020100);
+  Entry entry = Entry::Tombstone(partition, 16, format, "K", 16, 0x03020100);
 
   auto result = entry.Write("K", {});
   EXPECT_EQ(Status::OK, result.status());
diff --git a/pw_kvs/key_value_store.cc b/pw_kvs/key_value_store.cc
index 2d557e32b..2e7dd0e99 100644
--- a/pw_kvs/key_value_store.cc
+++ b/pw_kvs/key_value_store.cc
@@ -39,7 +39,7 @@ constexpr bool InvalidKey(std::string_view key) {
 KeyValueStore::KeyValueStore(FlashPartition* partition,
                              Vector<KeyDescriptor>& key_descriptor_list,
                              Vector<SectorDescriptor>& sector_descriptor_list,
-                             const EntryHeaderFormat& format,
+                             const EntryFormat& format,
                              const Options& options)
     : partition_(*partition),
       entry_header_format_(format),
@@ -740,16 +740,14 @@ KeyValueStore::Entry KeyValueStore::CreateEntry(Address address,
   if (state == KeyDescriptor::kDeleted) {
     return Entry::Tombstone(partition_,
                             address,
-                            entry_header_format_.magic,
-                            entry_header_format_.checksum,
+                            entry_header_format_,
                             key,
                             partition_.alignment_bytes(),
                             last_transaction_id_ + 1);
   }
   return Entry::Valid(partition_,
                       address,
-                      entry_header_format_.magic,
-                      entry_header_format_.checksum,
+                      entry_header_format_,
                       key,
                       value,
                       partition_.alignment_bytes(),
diff --git a/pw_kvs/key_value_store_fuzz_test.cc b/pw_kvs/key_value_store_fuzz_test.cc
index 45dc0cc6b..ea30bd914 100644
--- a/pw_kvs/key_value_store_fuzz_test.cc
+++ b/pw_kvs/key_value_store_fuzz_test.cc
@@ -30,7 +30,7 @@ FakeFlashBuffer<4 * 1024, 4> test_flash(16);
 FlashPartition test_partition(&test_flash, 0, test_flash.sector_count());
 
 ChecksumCrc16 checksum;
-constexpr EntryHeaderFormat kFormat{.magic = 0xBAD'C0D3, .checksum = &checksum};
+constexpr EntryFormat kFormat{.magic = 0xBAD'C0D3, .checksum = &checksum};
 
 class EmptyInitializedKvs : public ::testing::Test {
  protected:
diff --git a/pw_kvs/key_value_store_map_test.cc b/pw_kvs/key_value_store_map_test.cc
index b7da5c523..a65ed2c49 100644
--- a/pw_kvs/key_value_store_map_test.cc
+++ b/pw_kvs/key_value_store_map_test.cc
@@ -71,8 +71,8 @@ std::set<T> difference(const std::set<T> lhs, const std::set<T> rhs) {
 template <const TestParameters& kParams>
 class KvsTester {
  public:
-  static constexpr EntryHeaderFormat kFormat{.magic = 0xBAD'C0D3,
-                                             .checksum = nullptr};
+  static constexpr EntryFormat kFormat{.magic = 0xBAD'C0D3,
+                                       .checksum = nullptr};
 
   KvsTester()
       : partition_(&flash_,
diff --git a/pw_kvs/key_value_store_test.cc b/pw_kvs/key_value_store_test.cc
index 7922e8000..136e5489c 100644
--- a/pw_kvs/key_value_store_test.cc
+++ b/pw_kvs/key_value_store_test.cc
@@ -167,7 +167,7 @@ std::array<byte, 512> buffer;
 constexpr std::array<const char*, 3> keys{"TestKey1", "Key2", "TestKey3"};
 
 ChecksumCrc16 checksum;
-constexpr EntryHeaderFormat format{.magic = 0xBAD'C0D3, .checksum = &checksum};
+constexpr EntryFormat format{.magic = 0xBAD'C0D3, .checksum = &checksum};
 
 size_t RoundUpForAlignment(size_t size) {
   return AlignUp(size, test_partition.alignment_bytes());
@@ -571,8 +571,7 @@ TEST(InMemoryKvs, WriteOneKeyMultipleTimes) {
     DBG("xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx");
 
     // Create and initialize the KVS.
-    constexpr EntryHeaderFormat format{.magic = 0xBAD'C0D3,
-                                       .checksum = nullptr};
+    constexpr EntryFormat format{.magic = 0xBAD'C0D3, .checksum = nullptr};
     KeyValueStoreBuffer<kMaxEntries, kMaxUsableSectors> kvs(&flash.partition,
                                                             format);
     ASSERT_OK(kvs.Init());
@@ -613,7 +612,7 @@ TEST(InMemoryKvs, WritingMultipleKeysIncreasesSize) {
   ASSERT_OK(flash.partition.Erase());
 
   // Create and initialize the KVS.
-  constexpr EntryHeaderFormat format{.magic = 0xBAD'C0D3, .checksum = nullptr};
+  constexpr EntryFormat format{.magic = 0xBAD'C0D3, .checksum = nullptr};
   KeyValueStoreBuffer<kMaxEntries, kMaxUsableSectors> kvs(&flash.partition,
                                                           format);
   ASSERT_OK(kvs.Init());
@@ -640,7 +639,7 @@ TEST(InMemoryKvs, WriteAndReadOneKey) {
   ASSERT_OK(flash.partition.Erase());
 
   // Create and initialize the KVS.
-  constexpr EntryHeaderFormat format{.magic = 0xBAD'C0D3, .checksum = nullptr};
+  constexpr EntryFormat format{.magic = 0xBAD'C0D3, .checksum = nullptr};
   KeyValueStoreBuffer<kMaxEntries, kMaxUsableSectors> kvs(&flash.partition,
                                                           format);
   ASSERT_OK(kvs.Init());
@@ -669,7 +668,7 @@ TEST(InMemoryKvs, Basic) {
   ASSERT_EQ(Status::OK, flash.partition.Erase());
 
   // Create and initialize the KVS.
-  constexpr EntryHeaderFormat format{.magic = 0xBAD'C0D3, .checksum = nullptr};
+  constexpr EntryFormat format{.magic = 0xBAD'C0D3, .checksum = nullptr};
   KeyValueStoreBuffer<kMaxEntries, kMaxUsableSectors> kvs(&flash.partition,
                                                           format);
   ASSERT_OK(kvs.Init());
diff --git a/pw_kvs/public/pw_kvs/format.h b/pw_kvs/public/pw_kvs/format.h
new file mode 100644
index 000000000..8a206c924
--- /dev/null
+++ b/pw_kvs/public/pw_kvs/format.h
@@ -0,0 +1,76 @@
+// Copyright 2020 The Pigweed Authors
+//
+// Licensed under the Apache License, Version 2.0 (the "License"); you may not
+// use this file except in compliance with the License. You may obtain a copy of
+// the License at
+//
+//     https://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+// WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+// License for the specific language governing permissions and limitations under
+// the License.
+#pragma once
+
+namespace pw::kvs {
+namespace internal {
+
+// Disk format of the header used for each key-value entry.
+struct EntryHeader {
+  uint32_t magic;
+
+  // The checksum of the entire entry, including the header, key, value, and
+  // zero-value padding bytes. The checksum is calculated as if the checksum
+  // field value was zero.
+  uint32_t checksum;
+
+  // Stores the alignment in 16-byte units, starting from 16. To calculate the
+  // number of bytes, add one to this number and multiply by 16.
+  uint8_t alignment_units;
+
+  // The length of the key in bytes. The key is not null terminated.
+  //  6 bits, 0:5 - key length - maximum 64 characters
+  //  2 bits, 6:7 - reserved
+  uint8_t key_length_bytes;
+
+  // Byte length of the value; maximum of 65534. The max uint16_t value (65535
+  // or 0xFFFF) is reserved to indicate this is a tombstone (deleted) entry.
+  uint16_t value_size_bytes;
+
+  // The transaction ID for this key. Monotonically increasing.
+  uint32_t transaction_id;
+};
+
+static_assert(sizeof(EntryHeader) == 16, "EntryHeader must not have padding");
+
+}  // namespace internal
+
+// The EntryFormat defines properties of KVS entries that use a particular magic
+// number.
+struct EntryFormat {
+  // Magic is a unique constant identifier for entries.
+  //
+  // Upon reading from an address in flash, the magic number facilitiates
+  // quickly differentiating between:
+  //
+  // - Reading erased data - typically 0xFF - from flash.
+  // - Reading corrupted data
+  // - Reading a valid entry
+  //
+  // When selecting a magic for your particular KVS, pick a random 32 bit
+  // integer rather than a human readable 4 bytes. This decreases the
+  // probability of a collision with a real string when scanning in the case of
+  // corruption. To generate such a number:
+  /*
+       $ python3 -c 'import random; print(hex(random.randint(0,2**32)))'
+       0xaf741757
+  */
+  uint32_t magic;
+
+  // The checksum algorithm is used to calculate checksums for KVS entries. If
+  // it is null, no checksum is used.
+  ChecksumAlgorithm* checksum;
+};
+
+}  // namespace pw::kvs
diff --git a/pw_kvs/public/pw_kvs/internal/entry.h b/pw_kvs/public/pw_kvs/internal/entry.h
index d898f5660..03df6ebf6 100644
--- a/pw_kvs/public/pw_kvs/internal/entry.h
+++ b/pw_kvs/public/pw_kvs/internal/entry.h
@@ -24,39 +24,12 @@
 #include "pw_kvs/alignment.h"
 #include "pw_kvs/checksum.h"
 #include "pw_kvs/flash_memory.h"
+#include "pw_kvs/format.h"
 #include "pw_kvs/internal/key_descriptor.h"
 #include "pw_span/span.h"
 
 namespace pw::kvs::internal {
 
-// Disk format of the header used for each key-value entry.
-struct EntryHeader {
-  uint32_t magic;
-
-  // The checksum of the entire entry, including the header, key, value, and
-  // zero-value padding bytes. The checksum is calculated as if the checksum
-  // field value was zero.
-  uint32_t checksum;
-
-  // Stores the alignment in 16-byte units, starting from 16. To calculate the
-  // number of bytes, add one to this number and multiply by 16.
-  uint8_t alignment_units;
-
-  // The length of the key in bytes. The key is not null terminated.
-  //  6 bits, 0:5 - key length - maximum 64 characters
-  //  2 bits, 6:7 - reserved
-  uint8_t key_length_bytes;
-
-  // Byte length of the value; maximum of 65534. The max uint16_t value (65535
-  // or 0xFFFF) is reserved to indicate this is a tombstone (deleted) entry.
-  uint16_t value_size_bytes;
-
-  // The transaction ID for this key. Monotonically increasing.
-  uint32_t transaction_id;
-};
-
-static_assert(sizeof(EntryHeader) == 16, "EntryHeader must not have padding");
-
 // Entry represents a key-value entry in a flash partition.
 class Entry {
  public:
@@ -85,17 +58,14 @@ class Entry {
   // Creates a new Entry for a valid (non-deleted) entry.
   static Entry Valid(FlashPartition& partition,
                      Address address,
-                     // TODO: Use EntryHeaderFormat here?
-                     uint32_t magic,
-                     ChecksumAlgorithm* algorithm,
+                     const EntryFormat& format,
                      std::string_view key,
                      span<const std::byte> value,
                      size_t alignment_bytes,
                      uint32_t transaction_id) {
     return Entry(partition,
                  address,
-                 magic,
-                 algorithm,
+                 format,
                  key,
                  value,
                  value.size(),
@@ -106,15 +76,13 @@ class Entry {
   // Creates a new Entry for a tombstone entry, which marks a deleted key.
   static Entry Tombstone(FlashPartition& partition,
                          Address address,
-                         uint32_t magic,
-                         ChecksumAlgorithm* algorithm,
+                         const EntryFormat& format,
                          std::string_view key,
                          size_t alignment_bytes,
                          uint32_t transaction_id) {
     return Entry(partition,
                  address,
-                 magic,
-                 algorithm,
+                 format,
                  key,
                  {},
                  kDeletedValueLength,
@@ -209,8 +177,7 @@ class Entry {
 
   Entry(FlashPartition& partition,
         Address address,
-        uint32_t magic,
-        ChecksumAlgorithm* algorithm,
+        const EntryFormat& format,
         std::string_view key,
         span<const std::byte> value,
         uint16_t value_size_bytes,
diff --git a/pw_kvs/public/pw_kvs/key_value_store.h b/pw_kvs/public/pw_kvs/key_value_store.h
index 71f433b74..14b6ae561 100644
--- a/pw_kvs/public/pw_kvs/key_value_store.h
+++ b/pw_kvs/public/pw_kvs/key_value_store.h
@@ -22,6 +22,7 @@
 #include "pw_containers/vector.h"
 #include "pw_kvs/checksum.h"
 #include "pw_kvs/flash_memory.h"
+#include "pw_kvs/format.h"
 #include "pw_kvs/internal/entry.h"
 #include "pw_kvs/internal/key_descriptor.h"
 #include "pw_kvs/internal/sector_descriptor.h"
@@ -51,28 +52,6 @@ template <typename T>
 using ConvertsToSpan =
     std::bool_constant<internal::ConvertsToSpan<std::remove_reference_t<T>>(0)>;
 
-struct EntryHeaderFormat {
-  // Magic is a unique constant identifier for entries.
-  //
-  // Upon reading from an address in flash, the magic number facilitiates
-  // quickly differentiating between:
-  //
-  // - Reading erased data - typically 0xFF - from flash.
-  // - Reading corrupted data
-  // - Reading a valid entry
-  //
-  // When selecting a magic for your particular KVS, pick a random 32 bit
-  // integer rather than a human readable 4 bytes. This decreases the
-  // probability of a collision with a real string when scanning in the case of
-  // corruption. To generate such a number:
-  /*
-       $ python3 -c 'import random; print(hex(random.randint(0,2**32)))'
-       0xaf741757
-  */
-  uint32_t magic;
-  ChecksumAlgorithm* checksum;
-};
-
 // TODO: Select the appropriate defaults, add descriptions.
 struct Options {
   bool partial_gc_on_write = true;
@@ -238,7 +217,7 @@ class KeyValueStore {
   KeyValueStore(FlashPartition* partition,
                 Vector<KeyDescriptor>& key_descriptor_list,
                 Vector<SectorDescriptor>& sector_descriptor_list,
-                const EntryHeaderFormat& format,
+                const EntryFormat& format,
                 const Options& options);
 
  private:
@@ -341,7 +320,7 @@ class KeyValueStore {
   void LogKeyDescriptor() const;
 
   FlashPartition& partition_;
-  const EntryHeaderFormat entry_header_format_;
+  const EntryFormat entry_header_format_;
 
   // Unordered list of KeyDescriptors. Finding a key requires scanning and
   // verifying a match by reading the actual entry.
@@ -376,7 +355,7 @@ template <size_t kMaxEntries, size_t kMaxUsableSectors>
 class KeyValueStoreBuffer : public KeyValueStore {
  public:
   KeyValueStoreBuffer(FlashPartition* partition,
-                      const EntryHeaderFormat& format,
+                      const EntryFormat& format,
                       const Options& options = {})
       : KeyValueStore(partition, key_descriptors_, sectors_, format, options) {}