pw_kvs: Initial API changes

- Use std::string_view for keys to avoid the need for null termination.
- Use std::span and std::byte for data.
- Use StatusWithSize and size_t.
- Other minor adjustments.

Change-Id: I10d2f0f47b386071ed4ecf81586decfcc99244cd
diff --git a/pw_kvs/BUILD b/pw_kvs/BUILD
index d64f529..53d5579 100644
--- a/pw_kvs/BUILD
+++ b/pw_kvs/BUILD
@@ -41,7 +41,6 @@
         "//pw_checksum",
         "//pw_log",
         "//pw_status",
-        "//pw_string",
     ],
 )
 
diff --git a/pw_kvs/BUILD.gn b/pw_kvs/BUILD.gn
index d298849..83b1fc4 100644
--- a/pw_kvs/BUILD.gn
+++ b/pw_kvs/BUILD.gn
@@ -40,7 +40,6 @@
   deps = [
     dir_pw_checksum,
     dir_pw_log,
-    dir_pw_string,
   ]
 }
 
diff --git a/pw_kvs/flash.cc b/pw_kvs/flash.cc
index dad5fab..fbbc604 100644
--- a/pw_kvs/flash.cc
+++ b/pw_kvs/flash.cc
@@ -26,8 +26,10 @@
 
 Status PaddedWrite(FlashPartition* partition,
                    FlashPartition::Address address,
-                   const uint8_t* buffer,
+                   const void* raw_buffer,
                    uint16_t size) {
+  const uint8_t* const buffer = static_cast<const uint8_t*>(raw_buffer);
+
   if (address % partition->GetAlignmentBytes() ||
       partition->GetAlignmentBytes() > cfg::kFlashUtilMaxAlignmentBytes) {
     return Status::INVALID_ARGUMENT;
@@ -52,9 +54,11 @@
 }
 
 Status UnalignedRead(FlashPartition* partition,
-                     uint8_t* buffer,
+                     void* raw_buffer,
                      FlashPartition::Address address,
                      uint16_t size) {
+  uint8_t* const buffer = static_cast<uint8_t*>(raw_buffer);
+
   if (address % partition->GetAlignmentBytes() ||
       partition->GetAlignmentBytes() > cfg::kFlashUtilMaxAlignmentBytes) {
     return Status::INVALID_ARGUMENT;
@@ -73,7 +77,7 @@
         !status.ok()) {
       return status;
     }
-    memcpy(&buffer[aligned_bytes], alignment_buffer, remaining_bytes);
+    std::memcpy(&buffer[aligned_bytes], alignment_buffer, remaining_bytes);
   }
   return Status::OK;
 }
diff --git a/pw_kvs/key_value_store.cc b/pw_kvs/key_value_store.cc
index b1a6505..6eefb84 100644
--- a/pw_kvs/key_value_store.cc
+++ b/pw_kvs/key_value_store.cc
@@ -44,16 +44,17 @@
 
 #include "pw_kvs/key_value_store.h"
 
+#include <algorithm>
 #include <cstring>
 
 #include "pw_checksum/ccitt_crc16.h"
 #include "pw_kvs/flash.h"
 #include "pw_log/log.h"
-#include "pw_string/string_builder.h"
-#include "pw_string/util.h"
 
 namespace pw::kvs {
 
+using std::byte;
+
 Status KeyValueStore::Enable() {
   // TODO: LOCK MUTEX
   if (enabled_) {
@@ -61,7 +62,7 @@
   }
 
   // Reset parameters.
-  memset(sector_space_remaining_, 0, sizeof(sector_space_remaining_));
+  std::memset(sector_space_remaining_, 0, sizeof(sector_space_remaining_));
   map_size_ = 0;
 
   // For now alignment is set to use partitions alignment.
@@ -191,10 +192,15 @@
         break;  // End of elements in sector
       }
 
-      CHECK(header.key_len <= kChunkKeyLengthMax);
-      static_assert(sizeof(temp_key_buffer_) >= (kChunkKeyLengthMax + 1u),
-                    "Key buffer must be at least big enough for a key and a "
-                    "nul-terminator.");
+      if (header.key_len > kChunkKeyLengthMax) {
+        PW_LOG_CRITICAL("Found key with invalid length %u; maximum is %u",
+                        header.key_len,
+                        static_cast<unsigned>(kChunkKeyLengthMax));
+        return Status::DATA_LOSS;
+      }
+      static_assert(sizeof(temp_key_buffer_) >= kChunkKeyLengthMax + 1,
+                    "Key buffer must be at least large enough for a key and "
+                    "null terminator");
 
       // Read key and add to map
       if (Status status =
@@ -206,12 +212,13 @@
         return status;
       }
       temp_key_buffer_[header.key_len] = '\0';
+      std::string_view key(temp_key_buffer_, header.key_len);
       bool is_erased = header.flags & kFlagsIsErasedMask;
 
-      KeyIndex index = FindKeyInMap(temp_key_buffer_);
+      KeyIndex index = FindKeyInMap(key);
       if (index == kListCapacityMax) {
-        if (Status status = AppendToMap(
-                temp_key_buffer_, address, header.chunk_len, is_erased);
+        if (Status status =
+                AppendToMap(key, address, header.chunk_len, is_erased);
             !status.ok()) {
           return status;
         }
@@ -240,18 +247,10 @@
   return Status::OK;
 }
 
-Status KeyValueStore::Get(const char* key,
-                          void* raw_value,
-                          uint16_t size,
+Status KeyValueStore::Get(const std::string_view& key,
+                          const span<byte>& value,
                           uint16_t offset) {
-  uint8_t* const value = reinterpret_cast<uint8_t*>(raw_value);
-
-  if (key == nullptr || value == nullptr) {
-    return Status::INVALID_ARGUMENT;
-  }
-
-  size_t key_len = string::Length(key, kChunkKeyLengthMax + 1u);
-  if (key_len == 0 || key_len > kChunkKeyLengthMax) {
+  if (InvalidKey(key)) {
     return Status::INVALID_ARGUMENT;
   }
 
@@ -281,52 +280,53 @@
   if (kChunkSyncValue != header.synchronize_token) {
     return Status::DATA_LOSS;
   }
-  if (size + offset > header.chunk_len) {
+  if (value.size() + offset > header.chunk_len) {
     PW_LOG_ERROR("Out of bounds read: offset(%u) + size(%u) > data_size(%u)",
                  offset,
-                 size,
+                 unsigned(value.size()),
                  header.chunk_len);
     return Status::INVALID_ARGUMENT;
   }
   if (Status status = UnalignedRead(
           &partition_,
-          value,
+          value.data(),
           key_map_[key_index].address + RoundUpForAlignment(sizeof(KvsHeader)) +
               RoundUpForAlignment(header.key_len) + offset,
-          size);
+          value.size());
       !status.ok()) {
     return status;
   }
 
   // Verify CRC only when full packet was read.
-  if (offset == 0 && size == header.chunk_len) {
-    uint16_t crc = CalculateCrc(key, key_len, value, size);
+  if (offset == 0 && value.size() == header.chunk_len) {
+    uint16_t crc = CalculateCrc(key, value);
     if (crc != header.crc) {
-      PW_LOG_ERROR("KVS CRC does not match for key=%s [expected %u, found %u]",
-                   key,
-                   header.crc,
-                   crc);
+      // TODO: Figure out how to print the key's string_view. For now, using %s
+      // with a maximum length (8 characters). This still could trigger a small
+      // out-of-bounds read.
+      PW_LOG_ERROR(
+          "KVS CRC does not match for key=%.8s [expected %u, found %u]",
+          key.data(),
+          header.crc,
+          crc);
       return Status::DATA_LOSS;
     }
   }
   return Status::OK;
 }
 
-uint16_t KeyValueStore::CalculateCrc(const char* key,
-                                     uint16_t key_size,
-                                     const uint8_t* value,
-                                     uint16_t value_size) const {
-  uint16_t crc = checksum::CcittCrc16(as_bytes(span(key, key_size)));
-  return checksum::CcittCrc16(as_bytes(span(value, value_size)), crc);
+uint16_t KeyValueStore::CalculateCrc(const std::string_view& key,
+                                     const span<const byte>& value) const {
+  uint16_t crc = checksum::CcittCrc16(as_bytes(span(key)));
+  return checksum::CcittCrc16(value, crc);
 }
 
 Status KeyValueStore::CalculateCrcFromValueAddress(
-    const char* key,
-    uint16_t key_size,
+    const std::string_view& key,
     FlashPartition::Address value_address,
     uint16_t value_size,
     uint16_t* crc_ret) {
-  uint16_t crc = checksum::CcittCrc16(as_bytes(span(key, key_size)));
+  uint16_t crc = checksum::CcittCrc16(as_bytes(span(key)));
   for (size_t i = 0; i < value_size; i += TempBufferAlignedSize()) {
     auto read_size = std::min(value_size - i, TempBufferAlignedSize());
     if (Status status = UnalignedRead(
@@ -340,17 +340,9 @@
   return Status::OK;
 }
 
-Status KeyValueStore::Put(const char* key,
-                          const void* raw_value,
-                          uint16_t size) {
-  const uint8_t* const value = reinterpret_cast<const uint8_t*>(raw_value);
-  if (key == nullptr || value == nullptr) {
-    return Status::INVALID_ARGUMENT;
-  }
-
-  size_t key_len = string::Length(key, kChunkKeyLengthMax + 1u);
-  if (key_len == 0 || key_len > kChunkKeyLengthMax ||
-      size > kChunkValueLengthMax) {
+Status KeyValueStore::Put(const std::string_view& key,
+                          const span<const byte>& value) {
+  if (InvalidKey(key)) {
     return Status::INVALID_ARGUMENT;
   }
 
@@ -361,10 +353,11 @@
 
   KeyIndex index = FindKeyInMap(key);
   if (index != kListCapacityMax) {  // Key already in map, rewrite value
-    return RewriteValue(index, value, size);
+    return RewriteValue(index, value);
   }
 
-  FlashPartition::Address address = FindSpace(ChunkSize(key_len, size));
+  FlashPartition::Address address =
+      FindSpace(ChunkSize(key.size(), value.size()));
   if (address == kSectorInvalid) {
     return Status::RESOURCE_EXHAUSTED;
   }
@@ -375,17 +368,17 @@
     if (Status status = FullGarbageCollect(); !status.ok()) {
       return status;
     }
-    address = FindSpace(ChunkSize(key_len, size));
+    address = FindSpace(ChunkSize(key.size(), value.size()));
     if (address == kSectorInvalid || IsInLastFreeSector(address)) {
       // Couldn't find space, KVS is full.
       return Status::RESOURCE_EXHAUSTED;
     }
   }
 
-  if (Status status = WriteKeyValue(address, key, value, size); !status.ok()) {
+  if (Status status = WriteKeyValue(address, key, value); !status.ok()) {
     return status;
   }
-  if (Status status = AppendToMap(key, address, size); !status.ok()) {
+  if (Status status = AppendToMap(key, address, value.size()); !status.ok()) {
     return status;
   }
 
@@ -415,7 +408,7 @@
       if (!status.ok() && status != Status::RESOURCE_EXHAUSTED) {
         return status;
       }
-      if (exit_when_have_free_sector && HaveEmptySectorImpl()) {
+      if (exit_when_have_free_sector && HasEmptySector()) {
         return Status::OK;  // Now have a free sector
       }
     }
@@ -442,7 +435,7 @@
       !status.ok()) {
     return status;
   }
-  return HaveEmptySectorImpl() ? Status::OK : Status::RESOURCE_EXHAUSTED;
+  return HasEmptySector() ? Status::OK : Status::RESOURCE_EXHAUSTED;
 }
 
 Status KeyValueStore::FullGarbageCollect() {
@@ -456,23 +449,20 @@
                             false /*exit_when_have_free_sector*/);
 }
 
-Status KeyValueStore::RewriteValue(KeyIndex key_index,
-                                   const uint8_t* value,
-                                   uint16_t size,
+Status KeyValueStore::RewriteValue(KeyIndex index,
+                                   const span<const byte>& value,
                                    bool is_erased) {
   // Compare values, return if values are the same.
-  if (ValueMatches(key_index, value, size, is_erased)) {
+  if (ValueMatches(index, value, is_erased)) {
     return Status::OK;
   }
 
-  size_t key_length =
-      string::Length(key_map_[key_index].key, kChunkKeyLengthMax + 1u);
-  if (key_length > kChunkKeyLengthMax) {
+  if (key_map_[index].key_length > kChunkKeyLengthMax) {
     return Status::INTERNAL;
   }
 
-  uint32_t space_required = ChunkSize(key_length, size);
-  SectorIndex sector = AddressToSectorIndex(key_map_[key_index].address);
+  uint32_t space_required = ChunkSize(key_map_[index].key_length, value.size());
+  SectorIndex sector = AddressToSectorIndex(key_map_[index].address);
   uint32_t sector_space_remaining = SectorSpaceRemaining(sector);
 
   FlashPartition::Address address = kSectorInvalid;
@@ -485,27 +475,26 @@
     if (Status status = MarkSectorForClean(sector); !status.ok()) {
       return status;
     }
-    address = FindSpace(ChunkSize(key_length, size));
+    address = FindSpace(ChunkSize(key_map_[index].key_length, value.size()));
   }
   if (address == kSectorInvalid) {
     return Status::RESOURCE_EXHAUSTED;
   }
-  if (Status status = WriteKeyValue(
-          address, key_map_[key_index].key, value, size, is_erased);
+  if (Status status =
+          WriteKeyValue(address, key_map_[index].key(), value, is_erased);
       !status.ok()) {
     return status;
   }
-  UpdateMap(key_index, address, size, is_erased);
+  UpdateMap(index, address, value.size(), is_erased);
 
   return EnforceFreeSector();
 }
 
 bool KeyValueStore::ValueMatches(KeyIndex index,
-                                 const uint8_t* value,
-                                 uint16_t size,
+                                 const span<const byte>& value,
                                  bool is_erased) {
   // Compare sizes of CRC.
-  if (size != key_map_[index].chunk_len) {
+  if (value.size() != key_map_[index].chunk_len) {
     return false;
   }
   KvsHeader header;
@@ -513,25 +502,25 @@
                 reinterpret_cast<uint8_t*>(&header),
                 key_map_[index].address,
                 sizeof(header));
-  uint8_t key_len =
-      string::Length(key_map_[index].key, kChunkKeyLengthMax + 1u);
-  if (key_len > kChunkKeyLengthMax) {
+  std::string_view key = key_map_[index].key();
+  if (InvalidKey(key)) {
     return false;
   }
 
   if ((header.flags & kFlagsIsErasedMask) != is_erased) {
     return false;
-  } else if ((header.flags & kFlagsIsErasedMask) && is_erased) {
+  }
+  if ((header.flags & kFlagsIsErasedMask) && is_erased) {
     return true;
   }
 
   // Compare checksums.
-  if (header.crc != CalculateCrc(key_map_[index].key, key_len, value, size)) {
+  if (header.crc != CalculateCrc(key_map_[index].key(), value)) {
     return false;
   }
   FlashPartition::Address address = key_map_[index].address +
                                     RoundUpForAlignment(sizeof(KvsHeader)) +
-                                    RoundUpForAlignment(key_len);
+                                    RoundUpForAlignment(key.size());
   // Compare full values.
   for (size_t i = 0; i < key_map_[index].chunk_len;
        i += TempBufferAlignedSize()) {
@@ -540,23 +529,18 @@
     auto status =
         UnalignedRead(&partition_, temp_buffer_, address + i, read_size);
     if (!status.ok()) {
-      PW_LOG_ERROR("Failed to read chunk: %s", status.str());
+      PW_LOG_ERROR("%s: Failed to read chunk", status.str());
       return false;
     }
-    if (memcmp(value + i, temp_buffer_, read_size) != 0) {
+    if (std::memcmp(&value[i], temp_buffer_, read_size) != 0) {
       return false;
     }
   }
   return true;
 }
 
-Status KeyValueStore::Erase(const char* key) {
-  if (key == nullptr) {
-    return Status::INVALID_ARGUMENT;
-  }
-
-  size_t key_len = string::Length(key, kChunkKeyLengthMax + 1u);
-  if (key_len == 0 || key_len > kChunkKeyLengthMax) {
+Status KeyValueStore::Erase(const std::string_view& key) {
+  if (InvalidKey(key)) {
     return Status::INVALID_ARGUMENT;
   }
   // TODO: LOCK MUTEX
@@ -568,7 +552,7 @@
   if (key_index == kListCapacityMax || key_map_[key_index].is_erased) {
     return Status::NOT_FOUND;
   }
-  return RewriteValue(key_index, nullptr, 0, true);
+  return RewriteValue(key_index, span<byte>(), true);
 }
 
 Status KeyValueStore::ResetSector(SectorIndex sector_index) {
@@ -604,22 +588,22 @@
 }
 
 Status KeyValueStore::WriteKeyValue(FlashPartition::Address address,
-                                    const char* key,
-                                    const uint8_t* value,
-                                    uint16_t size,
+                                    const std::string_view& key,
+                                    const span<const byte>& value,
                                     bool is_erased) {
-  uint16_t key_length = string::Length(key, kChunkKeyLengthMax + 1u);
-  if (key_length > kChunkKeyLengthMax) {
+  if (InvalidKey(key) ||
+      value.size() >
+          std::numeric_limits<decltype(KvsHeader::chunk_len)>::max()) {
     return Status::INTERNAL;
   }
 
   constexpr uint16_t kFlagDefaultValue = 0;
   KvsHeader header = {
       .synchronize_token = kChunkSyncValue,
-      .crc = CalculateCrc(key, key_length, value, size),
+      .crc = CalculateCrc(key, value),
       .flags = is_erased ? kFlagsIsErasedMask : kFlagDefaultValue,
-      .key_len = key_length,
-      .chunk_len = size};
+      .key_len = static_cast<uint8_t>(key.size()),
+      .chunk_len = static_cast<uint16_t>(value.size())};
 
   SectorIndex sector = AddressToSectorIndex(address);
   if (Status status = PaddedWrite(&partition_,
@@ -632,18 +616,19 @@
   address += RoundUpForAlignment(sizeof(header));
   if (Status status = PaddedWrite(&partition_,
                                   address,
-                                  reinterpret_cast<const uint8_t*>(key),
-                                  key_length);
+                                  reinterpret_cast<const uint8_t*>(key.data()),
+                                  key.size());
       !status.ok()) {
   }
-  address += RoundUpForAlignment(key_length);
-  if (size > 0) {
-    if (Status status = PaddedWrite(&partition_, address, value, size);
-        !status.ok()) {
+  address += RoundUpForAlignment(key.size());
+  if (!value.empty()) {
+    Status status =
+        PaddedWrite(&partition_, address, value.data(), value.size());
+    if (!status.ok()) {
       return status;
     }
   }
-  sector_space_remaining_[sector] -= ChunkSize(key_length, size);
+  sector_space_remaining_[sector] -= ChunkSize(key.size(), value.size());
   return Status::OK;
 }
 
@@ -709,10 +694,8 @@
     }
 
     if (i < map_size_ && sector == AddressToSectorIndex(key_map_[i].address)) {
-      uint8_t key_len =
-          string::Length(key_map_[i].key, kChunkKeyLengthMax + 1u);
       FlashPartition::Address address = key_map_[i].address;
-      auto size = ChunkSize(key_len, key_map_[i].chunk_len);
+      auto size = ChunkSize(key_map_[i].key_length, key_map_[i].chunk_len);
       FlashPartition::Address move_address = FindSpace(size);
       if (move_address == kSectorInvalid) {
         return Status::RESOURCE_EXHAUSTED;
@@ -762,8 +745,7 @@
   return Status::OK;
 }
 
-FlashPartition::Address KeyValueStore::FindSpace(
-    uint16_t requested_size) const {
+FlashPartition::Address KeyValueStore::FindSpace(size_t requested_size) const {
   if (requested_size > SectorSpaceAvailableWhenEmpty()) {
     return kSectorInvalid;  // This would never fit
   }
@@ -794,58 +776,49 @@
   return sector_space_remaining_[sector_index];
 }
 
-Status KeyValueStore::GetValueSize(const char* key, uint16_t* value_size) {
-  if (key == nullptr || value_size == nullptr) {
-    return Status::INVALID_ARGUMENT;
+StatusWithSize KeyValueStore::GetValueSize(const std::string_view& key) {
+  if (InvalidKey(key)) {
+    return StatusWithSize(Status::INVALID_ARGUMENT, 0);
   }
 
-  size_t key_len = string::Length(key, kChunkKeyLengthMax + 2u);
-  if (key_len == 0 || key_len > kChunkKeyLengthMax) {
-    return Status::INVALID_ARGUMENT;
-  }
   // TODO: LOCK MUTEX
   if (!enabled_) {
-    return Status::FAILED_PRECONDITION;
+    return StatusWithSize(Status::FAILED_PRECONDITION, 0);
   }
 
   uint8_t idx = FindKeyInMap(key);
   if (idx == kListCapacityMax || key_map_[idx].is_erased) {
-    return Status::NOT_FOUND;
+    return StatusWithSize(Status::NOT_FOUND, 0);
   }
-  *value_size = key_map_[idx].chunk_len;
-  return Status::OK;
+  return StatusWithSize(key_map_[idx].chunk_len);
 }
 
-Status KeyValueStore::AppendToMap(const char* key,
+Status KeyValueStore::AppendToMap(const std::string_view& key,
                                   FlashPartition::Address address,
-                                  uint16_t chunk_len,
+                                  size_t chunk_len,
                                   bool is_erased) {
   if (map_size_ >= kListCapacityMax) {
     PW_LOG_ERROR("Can't add: reached max supported keys %d", kListCapacityMax);
     return Status::INTERNAL;
   }
 
-  // Copy incoming key into map entry, ensuring size checks and nul-termination.
-  StringBuilder key_builder(
-      span(key_map_[map_size_].key, sizeof(key_map_[map_size_].key)));
-  key_builder.append(key);
+  auto& entry = key_map_[map_size_];
+  entry.key_buffer[key.copy(entry.key_buffer, sizeof(KeyMap::key_buffer) - 1)] =
+      '\0';
 
-  if (!key_builder.status().ok()) {
-    PW_LOG_ERROR("Can't add: got invalid key: %s!", key_builder.status().str());
-    return Status::INTERNAL;
-  }
-
-  key_map_[map_size_].address = address;
-  key_map_[map_size_].chunk_len = chunk_len;
-  key_map_[map_size_].is_erased = is_erased;
+  entry.key_length = key.size();
+  entry.address = address;
+  entry.chunk_len = static_cast<uint16_t>(chunk_len);
+  entry.is_erased = is_erased;
   map_size_++;
 
   return Status::OK;
 }
 
-KeyValueStore::KeyIndex KeyValueStore::FindKeyInMap(const char* key) const {
+KeyValueStore::KeyIndex KeyValueStore::FindKeyInMap(
+    const std::string_view& key) const {
   for (KeyIndex i = 0; i < map_size_; i++) {
-    if (strncmp(key, key_map_[i].key, sizeof(key_map_[i].key)) == 0) {
+    if (key == std::string_view(key_map_[i].key())) {
       return i;
     }
   }
@@ -865,29 +838,29 @@
   key_map_[key_index] = key_map_[--map_size_];
 }
 
-uint8_t KeyValueStore::KeyCount() const {
-  uint8_t count = 0;
+size_t KeyValueStore::KeyCount() const {
+  size_t count = 0;
   for (unsigned i = 0; i < map_size_; i++) {
     count += key_map_[i].is_erased ? 0 : 1;
   }
   return count;
 }
 
-const char* KeyValueStore::GetKey(uint8_t idx) const {
-  uint8_t count = 0;
+std::string_view KeyValueStore::GetKey(size_t idx) const {
+  unsigned count = 0;
   for (unsigned i = 0; i < map_size_; i++) {
     if (!key_map_[i].is_erased) {
       if (idx == count) {
-        return key_map_[i].key;
+        return key_map_[i].key();
       }
       count++;
     }
   }
-  return nullptr;
+  return {};
 }
 
-uint16_t KeyValueStore::GetValueSize(uint8_t idx) const {
-  uint8_t count = 0;
+size_t KeyValueStore::GetValueSize(size_t idx) const {
+  size_t count = 0;
   for (unsigned i = 0; i < map_size_; i++) {
     if (!key_map_[i].is_erased) {
       if (idx == count++) {
diff --git a/pw_kvs/key_value_store_test.cc b/pw_kvs/key_value_store_test.cc
index 92bc6a5..5d652dd 100644
--- a/pw_kvs/key_value_store_test.cc
+++ b/pw_kvs/key_value_store_test.cc
@@ -14,6 +14,9 @@
 
 #include "pw_kvs/key_value_store.h"
 
+#include <array>
+#include <cstdio>
+
 #include "pw_span/span.h"
 
 #define USE_MEMORY_BUFFER 1
@@ -32,6 +35,17 @@
 namespace pw::kvs {
 namespace {
 
+using std::byte;
+
+// Test that the IsSpan trait correctly idenitifies span instantiations.
+static_assert(!internal::IsSpan<int>());
+static_assert(!internal::IsSpan<void>());
+static_assert(!internal::IsSpan<std::array<int, 5>>());
+
+static_assert(internal::IsSpan<span<int>>());
+static_assert(internal::IsSpan<span<byte>>());
+static_assert(internal::IsSpan<span<const int*>>());
+
 #if USE_MEMORY_BUFFER
 // Although it might be useful to test other configurations, some tests require
 // at least 3 sectors; therfore it should have this when checked in.
@@ -56,8 +70,8 @@
   KeyValueStore kvs_local_;
 };
 
-std::array<uint8_t, 512> buffer;
-std::array<const char*, 3> keys{"TestKey1", "Key2", "TestKey3"};
+std::array<byte, 512> buffer;
+constexpr std::array<const char*, 3> keys{"TestKey1", "Key2", "TestKey3"};
 
 Status PaddedWrite(FlashPartition* partition,
                    FlashPartition::Address address,
@@ -72,7 +86,7 @@
   }
   uint16_t remaining_bytes = size - aligned_bytes;
   if (remaining_bytes > 0) {
-    memcpy(alignment_buffer, &buf[aligned_bytes], remaining_bytes);
+    std::memcpy(alignment_buffer, &buf[aligned_bytes], remaining_bytes);
     if (Status status = partition->Write(address + aligned_bytes,
                                          alignment_buffer,
                                          partition->GetAlignmentBytes());
@@ -142,14 +156,15 @@
   // the end.
   size_t chunk_len =
       std::max(kvs_attr.MinPutSize(), size_to_fill % buffer.size());
-  memset(buffer.data(), 0, buffer.size());
+  std::memset(buffer.data(), 0, buffer.size());
   while (size_to_fill > 0) {
-    buffer[0]++;  // Changing buffer value so put actually does something
-    ASSERT_EQ(
-        Status::OK,
-        kvs.Put(key,
-                buffer.data(),
-                chunk_len - kvs_attr.ChunkHeaderSize() - kvs_attr.KeySize()));
+    // Changing buffer value so put actually does something
+    buffer[0] = static_cast<byte>(static_cast<uint8_t>(buffer[0]) + 1);
+    ASSERT_EQ(Status::OK,
+              kvs.Put(key,
+                      span(buffer.data(),
+                           chunk_len - kvs_attr.ChunkHeaderSize() -
+                               kvs_attr.KeySize())));
     size_to_fill -= chunk_len;
     chunk_len = std::min(size_to_fill, kMaxPutSize);
   }
@@ -160,8 +175,8 @@
   static constexpr size_t kChunkKeyLengthMax = 15;
   uint16_t crc = checksum::CcittCrc16(
       as_bytes(span(key, string::Length(key, kChunkKeyLengthMax))));
-  return checksum::CcittCrc16(
-      span(static_cast<const std::byte*>(data), data_len), crc);
+  return checksum::CcittCrc16(span(static_cast<const byte*>(data), data_len),
+                              crc);
 }
 
 uint16_t CalcTestPartitionCrc() {
@@ -193,14 +208,14 @@
   const char* key1 = "Buf1";
   const char* key2 = "Buf2";
   const size_t kLargestBufSize = 3 * 1024;
-  static uint8_t buf1[kLargestBufSize];
-  static uint8_t buf2[kLargestBufSize];
-  memset(buf1, 1, sizeof(buf1));
-  memset(buf2, 2, sizeof(buf2));
+  static byte buf1[kLargestBufSize];
+  static byte buf2[kLargestBufSize];
+  std::memset(buf1, 1, sizeof(buf1));
+  std::memset(buf2, 2, sizeof(buf2));
 
   // Start with things in KVS
-  ASSERT_EQ(Status::OK, kvs.Put(key1, buf1, sizeof(buf1)));
-  ASSERT_EQ(Status::OK, kvs.Put(key2, buf2, sizeof(buf2)));
+  ASSERT_EQ(Status::OK, kvs.Put(key1, buf1));
+  ASSERT_EQ(Status::OK, kvs.Put(key2, buf2));
   for (size_t j = 0; j < keys.size(); j++) {
     ASSERT_EQ(Status::OK, kvs.Put(keys[j], j));
   }
@@ -215,9 +230,9 @@
     }
     // Delete and re-add everything
     ASSERT_EQ(Status::OK, kvs.Erase(key1));
-    ASSERT_EQ(Status::OK, kvs.Put(key1, buf1, size1));
+    ASSERT_EQ(Status::OK, kvs.Put(key1, span(buf1, size1)));
     ASSERT_EQ(Status::OK, kvs.Erase(key2));
-    ASSERT_EQ(Status::OK, kvs.Put(key2, buf2, size2));
+    ASSERT_EQ(Status::OK, kvs.Put(key2, span(buf2, size2)));
     for (size_t j = 0; j < keys.size(); j++) {
       ASSERT_EQ(Status::OK, kvs.Erase(keys[j]));
       ASSERT_EQ(Status::OK, kvs.Put(keys[j], j));
@@ -226,11 +241,11 @@
     // Re-enable and verify
     kvs.Disable();
     ASSERT_EQ(Status::OK, kvs.Enable());
-    static uint8_t buf[4 * 1024];
-    ASSERT_EQ(Status::OK, kvs.Get(key1, buf, size1));
-    ASSERT_EQ(memcmp(buf, buf1, size1), 0);
-    ASSERT_EQ(Status::OK, kvs.Get(key2, buf, size2));
-    ASSERT_EQ(memcmp(buf2, buf2, size2), 0);
+    static byte buf[4 * 1024];
+    ASSERT_EQ(Status::OK, kvs.Get(key1, span(buf, size1)));
+    ASSERT_EQ(std::memcmp(buf, buf1, size1), 0);
+    ASSERT_EQ(Status::OK, kvs.Get(key2, span(buf, size2)));
+    ASSERT_EQ(std::memcmp(buf2, buf2, size2), 0);
     for (size_t j = 0; j < keys.size(); j++) {
       size_t ret = 1000;
       ASSERT_EQ(Status::OK, kvs.Get(keys[j], &ret));
@@ -250,22 +265,17 @@
 
   // Add some data
   uint8_t value1 = 0xDA;
-  ASSERT_EQ(Status::OK, kvs.Put(keys[0], &value1, sizeof(value1)));
+  ASSERT_EQ(Status::OK,
+            kvs.Put(keys[0], as_bytes(span(&value1, sizeof(value1)))));
 
   uint32_t value2 = 0xBAD0301f;
-  ASSERT_EQ(
-      Status::OK,
-      kvs.Put(keys[1], reinterpret_cast<uint8_t*>(&value2), sizeof(value2)));
+  ASSERT_EQ(Status::OK, kvs.Put(keys[1], value2));
 
   // Verify data
   uint32_t test2;
-  EXPECT_EQ(
-      Status::OK,
-      kvs.Get(keys[1], reinterpret_cast<uint8_t*>(&test2), sizeof(test2)));
+  EXPECT_EQ(Status::OK, kvs.Get(keys[1], &test2));
   uint8_t test1;
-  ASSERT_EQ(
-      Status::OK,
-      kvs.Get(keys[0], reinterpret_cast<uint8_t*>(&test1), sizeof(test1)));
+  ASSERT_EQ(Status::OK, kvs.Get(keys[0], &test1));
 
   EXPECT_EQ(test1, value1);
   EXPECT_EQ(test2, value2);
@@ -274,19 +284,18 @@
   EXPECT_EQ(Status::OK, kvs.Erase(keys[0]));
 
   // Verify it was erased
-  EXPECT_EQ(kvs.Get(keys[0], reinterpret_cast<uint8_t*>(&test1), sizeof(test1)),
-            Status::NOT_FOUND);
+  EXPECT_EQ(kvs.Get(keys[0], &test1), Status::NOT_FOUND);
   test2 = 0;
   ASSERT_EQ(
       Status::OK,
-      kvs.Get(keys[1], reinterpret_cast<uint8_t*>(&test2), sizeof(test2)));
+      kvs.Get(keys[1], span(reinterpret_cast<byte*>(&test2), sizeof(test2))));
   EXPECT_EQ(test2, value2);
 
   // Erase other key
   kvs.Erase(keys[1]);
 
   // Verify it was erased
-  EXPECT_EQ(kvs.KeyCount(), 0);
+  EXPECT_EQ(kvs.KeyCount(), 0u);
 }
 
 TEST_F(KeyValueStoreTest, MaxKeyLength) {
@@ -342,15 +351,14 @@
 
   // Add and verify
   for (unsigned add_idx = 0; add_idx < keys.size(); add_idx++) {
-    memset(buffer.data(), add_idx, buffer.size());
-    ASSERT_EQ(Status::OK, kvs.Put(keys[add_idx], buffer.data(), buffer.size()));
+    std::memset(buffer.data(), add_idx, buffer.size());
+    ASSERT_EQ(Status::OK, kvs.Put(keys[add_idx], buffer));
     EXPECT_EQ(kvs.KeyCount(), add_idx + 1);
     for (unsigned verify_idx = 0; verify_idx <= add_idx; verify_idx++) {
-      memset(buffer.data(), 0, buffer.size());
-      ASSERT_EQ(Status::OK,
-                kvs.Get(keys[verify_idx], buffer.data(), buffer.size()));
+      std::memset(buffer.data(), 0, buffer.size());
+      ASSERT_EQ(Status::OK, kvs.Get(keys[verify_idx], buffer));
       for (unsigned i = 0; i < buffer.size(); i++) {
-        EXPECT_EQ(buffer[i], verify_idx);
+        EXPECT_EQ(static_cast<unsigned>(buffer[i]), verify_idx);
       }
     }
   }
@@ -360,15 +368,13 @@
     ASSERT_EQ(Status::OK, kvs.Erase(keys[erase_idx]));
     EXPECT_EQ(kvs.KeyCount(), keys.size() - erase_idx - 1);
     for (unsigned verify_idx = 0; verify_idx < keys.size(); verify_idx++) {
-      memset(buffer.data(), 0, buffer.size());
+      std::memset(buffer.data(), 0, buffer.size());
       if (verify_idx <= erase_idx) {
-        ASSERT_EQ(kvs.Get(keys[verify_idx], buffer.data(), buffer.size()),
-                  Status::NOT_FOUND);
+        ASSERT_EQ(kvs.Get(keys[verify_idx], buffer), Status::NOT_FOUND);
       } else {
-        ASSERT_EQ(Status::OK,
-                  kvs.Get(keys[verify_idx], buffer.data(), buffer.size()));
+        ASSERT_EQ(Status::OK, kvs.Get(keys[verify_idx], buffer));
         for (uint32_t i = 0; i < buffer.size(); i++) {
-          EXPECT_EQ(buffer[i], verify_idx);
+          EXPECT_EQ(buffer[i], static_cast<byte>(verify_idx));
         }
       }
     }
@@ -400,8 +406,8 @@
 
   // Add some items
   for (unsigned add_idx = 0; add_idx < keys.size(); add_idx++) {
-    memset(buffer.data(), add_idx, buffer.size());
-    ASSERT_EQ(Status::OK, kvs.Put(keys[add_idx], buffer.data(), buffer.size()));
+    std::memset(buffer.data(), add_idx, buffer.size());
+    ASSERT_EQ(Status::OK, kvs.Put(keys[add_idx], buffer));
     EXPECT_EQ(kvs.KeyCount(), add_idx + 1);
   }
 
@@ -413,19 +419,18 @@
   // Ensure adding to new KVS works
   uint8_t value = 0xDA;
   const char* key = "new_key";
-  ASSERT_EQ(Status::OK, kvs_local_.Put(key, &value, sizeof(value)));
+  ASSERT_EQ(Status::OK, kvs_local_.Put(key, value));
   uint8_t test;
-  ASSERT_EQ(Status::OK, kvs_local_.Get(key, &test, sizeof(test)));
+  ASSERT_EQ(Status::OK, kvs_local_.Get(key, &test));
   EXPECT_EQ(value, test);
   EXPECT_EQ(kvs_local_.KeyCount(), keys.size() + 1);
 
   // Verify previous data
   for (unsigned verify_idx = 0; verify_idx < keys.size(); verify_idx++) {
-    memset(buffer.data(), 0, buffer.size());
-    ASSERT_EQ(Status::OK,
-              kvs_local_.Get(keys[verify_idx], buffer.data(), buffer.size()));
+    std::memset(buffer.data(), 0, buffer.size());
+    ASSERT_EQ(Status::OK, kvs_local_.Get(keys[verify_idx], buffer));
     for (uint32_t i = 0; i < buffer.size(); i++) {
-      EXPECT_EQ(buffer[i], verify_idx);
+      EXPECT_EQ(static_cast<unsigned>(buffer[i]), verify_idx);
     }
   }
 }
@@ -452,18 +457,18 @@
 
   char key[20];
   for (unsigned add_idx = 0; add_idx < add_count; add_idx++) {
-    memset(buffer.data(), add_idx, buffer.size());
+    std::memset(buffer.data(), add_idx, buffer.size());
     snprintf(key, sizeof(key), "key_%u", add_idx);
-    ASSERT_EQ(Status::OK, kvs.Put(key, buffer.data(), buffer.size()));
+    ASSERT_EQ(Status::OK, kvs.Put(key, buffer));
     EXPECT_EQ(kvs.KeyCount(), add_idx + 1);
   }
 
   for (unsigned verify_idx = 0; verify_idx < add_count; verify_idx++) {
-    memset(buffer.data(), 0, buffer.size());
+    std::memset(buffer.data(), 0, buffer.size());
     snprintf(key, sizeof(key), "key_%u", verify_idx);
-    ASSERT_EQ(Status::OK, kvs.Get(key, buffer.data(), buffer.size()));
+    ASSERT_EQ(Status::OK, kvs.Get(key, buffer));
     for (uint32_t i = 0; i < buffer.size(); i++) {
-      EXPECT_EQ(buffer[i], verify_idx);
+      EXPECT_EQ(static_cast<unsigned>(buffer[i]), verify_idx);
     }
   }
 
@@ -486,24 +491,22 @@
   const uint8_t kValue1 = 0xDA;
   const uint8_t kValue2 = 0x12;
   const char* key = "the_key";
-  ASSERT_EQ(Status::OK, kvs.Put(key, &kValue1, sizeof(kValue1)));
+  ASSERT_EQ(Status::OK, kvs.Put(key, as_bytes(span(&kValue1, 1))));
 
   // Verify
   uint8_t value;
-  ASSERT_EQ(Status::OK,
-            kvs.Get(key, reinterpret_cast<uint8_t*>(&value), sizeof(value)));
+  ASSERT_EQ(Status::OK, kvs.Get(key, as_writable_bytes(span(&value, 1))));
   EXPECT_EQ(kValue1, value);
 
   // Write new value for key
-  ASSERT_EQ(Status::OK, kvs.Put(key, &kValue2, sizeof(kValue2)));
+  ASSERT_EQ(Status::OK, kvs.Put(key, as_bytes(span(&kValue2, 1))));
 
   // Verify
-  ASSERT_EQ(Status::OK,
-            kvs.Get(key, reinterpret_cast<uint8_t*>(&value), sizeof(value)));
+  ASSERT_EQ(Status::OK, kvs.Get(key, as_writable_bytes(span(&value, 1))));
   EXPECT_EQ(kValue2, value);
 
   // Verify only 1 element exists
-  EXPECT_EQ(kvs.KeyCount(), 1);
+  EXPECT_EQ(kvs.KeyCount(), 1u);
 }
 
 TEST_F(KeyValueStoreTest, OffsetRead) {
@@ -514,25 +517,25 @@
   ASSERT_EQ(Status::OK, kvs.Enable());
 
   const char* key = "the_key";
-  constexpr uint8_t kReadSize = 16;  // needs to be a multiple of alignment
-  constexpr uint8_t kTestBufferSize = kReadSize * 10;
+  constexpr size_t kReadSize = 16;  // needs to be a multiple of alignment
+  constexpr size_t kTestBufferSize = kReadSize * 10;
   CHECK_GT(buffer.size(), kTestBufferSize);
   CHECK_LE(kTestBufferSize, 0xFF);
 
   // Write the entire buffer
-  for (uint8_t i = 0; i < kTestBufferSize; i++) {
-    buffer[i] = i;
+  for (size_t i = 0; i < kTestBufferSize; i++) {
+    buffer[i] = byte(i);
   }
-  ASSERT_EQ(Status::OK, kvs.Put(key, buffer.data(), kTestBufferSize));
-  EXPECT_EQ(kvs.KeyCount(), 1);
+  ASSERT_EQ(Status::OK, kvs.Put(key, span(buffer.data(), kTestBufferSize)));
+  EXPECT_EQ(kvs.KeyCount(), 1u);
 
   // Read in small chunks and verify
-  for (int i = 0; i < kTestBufferSize / kReadSize; i++) {
-    memset(buffer.data(), 0, buffer.size());
+  for (unsigned i = 0; i < kTestBufferSize / kReadSize; i++) {
+    std::memset(buffer.data(), 0, buffer.size());
     ASSERT_EQ(Status::OK,
-              kvs.Get(key, buffer.data(), kReadSize, i * kReadSize));
+              kvs.Get(key, span(buffer.data(), kReadSize), i * kReadSize));
     for (unsigned j = 0; j < kReadSize; j++) {
-      ASSERT_EQ(buffer[j], j + i * kReadSize);
+      ASSERT_EQ(static_cast<unsigned>(buffer[j]), j + i * kReadSize);
     }
   }
 }
@@ -552,20 +555,20 @@
   const char* key = "the_key";
   constexpr uint8_t kGoodVal = 0x60;
   constexpr uint8_t kBadVal = 0xBA;
-  memset(buffer.data(), kBadVal, buffer.size());
+  std::memset(buffer.data(), kBadVal, buffer.size());
   for (unsigned add_idx = 0; add_idx < add_count; add_idx++) {
     if (add_idx == add_count - 1) {  // last value
-      memset(buffer.data(), kGoodVal, buffer.size());
+      std::memset(buffer.data(), kGoodVal, buffer.size());
     }
-    ASSERT_EQ(Status::OK, kvs.Put(key, buffer.data(), buffer.size()));
-    EXPECT_EQ(kvs.KeyCount(), 1);
+    ASSERT_EQ(Status::OK, kvs.Put(key, buffer));
+    EXPECT_EQ(kvs.KeyCount(), 1u);
   }
 
   // Verify
-  memset(buffer.data(), 0, buffer.size());
-  ASSERT_EQ(Status::OK, kvs.Get(key, buffer.data(), buffer.size()));
+  std::memset(buffer.data(), 0, buffer.size());
+  ASSERT_EQ(Status::OK, kvs.Get(key, buffer));
   for (uint32_t i = 0; i < buffer.size(); i++) {
-    ASSERT_EQ(buffer[i], kGoodVal);
+    ASSERT_EQ(buffer[i], static_cast<byte>(kGoodVal));
   }
 }
 
@@ -584,26 +587,30 @@
   KvsAttributes kvs_attr(std::strlen(keys[2]), kTestDataSize);
   int bytes_remaining =
       test_partition.GetSectorSizeBytes() - kvs_attr.SectorHeaderSize();
-  constexpr uint8_t kKey0Pattern = 0xBA;
+  constexpr byte kKey0Pattern = byte{0xBA};
 
-  memset(buffer.data(), kKey0Pattern, kvs_attr.DataSize());
-  ASSERT_EQ(Status::OK, kvs.Put(keys[0], buffer.data(), kvs_attr.DataSize()));
+  std::memset(
+      buffer.data(), static_cast<int>(kKey0Pattern), kvs_attr.DataSize());
+  ASSERT_EQ(Status::OK,
+            kvs.Put(keys[0], span(buffer.data(), kvs_attr.DataSize())));
   bytes_remaining -= kvs_attr.MinPutSize();
-  memset(buffer.data(), 1, kvs_attr.DataSize());
-  ASSERT_EQ(Status::OK, kvs.Put(keys[2], buffer.data(), kvs_attr.DataSize()));
+  std::memset(buffer.data(), 1, kvs_attr.DataSize());
+  ASSERT_EQ(Status::OK,
+            kvs.Put(keys[2], span(buffer.data(), kvs_attr.DataSize())));
   bytes_remaining -= kvs_attr.MinPutSize();
-  EXPECT_EQ(kvs.KeyCount(), 2);
+  EXPECT_EQ(kvs.KeyCount(), 2u);
   ASSERT_EQ(Status::OK, kvs.Erase(keys[2]));
   bytes_remaining -= kvs_attr.EraseSize();
-  EXPECT_EQ(kvs.KeyCount(), 1);
+  EXPECT_EQ(kvs.KeyCount(), 1u);
 
   // Intentionally adding erase size to trigger sector cleanup
   bytes_remaining += kvs_attr.EraseSize();
   FillKvs(keys[2], bytes_remaining);
 
   // Verify key[0]
-  memset(buffer.data(), 0, kvs_attr.DataSize());
-  ASSERT_EQ(Status::OK, kvs.Get(keys[0], buffer.data(), kvs_attr.DataSize()));
+  std::memset(buffer.data(), 0, kvs_attr.DataSize());
+  ASSERT_EQ(Status::OK,
+            kvs.Get(keys[0], span(buffer.data(), kvs_attr.DataSize())));
   for (uint32_t i = 0; i < kvs_attr.DataSize(); i++) {
     EXPECT_EQ(buffer[i], kKey0Pattern);
   }
@@ -618,18 +625,18 @@
 
   const uint8_t kValue1 = 0xDA;
   const uint8_t kValue2 = 0x12;
-  uint8_t value[1];
-  ASSERT_EQ(Status::OK, kvs.Put(keys[0], &kValue1, sizeof(kValue1)));
-  EXPECT_EQ(kvs.KeyCount(), 1);
+  uint8_t value;
+  ASSERT_EQ(Status::OK, kvs.Put(keys[0], kValue1));
+  EXPECT_EQ(kvs.KeyCount(), 1u);
   ASSERT_EQ(Status::OK, kvs.Erase(keys[0]));
-  EXPECT_EQ(kvs.Get(keys[0], value, sizeof(value)), Status::NOT_FOUND);
-  ASSERT_EQ(Status::OK, kvs.Put(keys[1], &kValue1, sizeof(kValue1)));
-  ASSERT_EQ(Status::OK, kvs.Put(keys[2], &kValue2, sizeof(kValue2)));
+  EXPECT_EQ(kvs.Get(keys[0], &value), Status::NOT_FOUND);
+  ASSERT_EQ(Status::OK, kvs.Put(keys[1], as_bytes(span(&kValue1, 1))));
+  ASSERT_EQ(Status::OK, kvs.Put(keys[2], kValue2));
   ASSERT_EQ(Status::OK, kvs.Erase(keys[1]));
-  EXPECT_EQ(Status::OK, kvs.Get(keys[2], value, sizeof(value)));
-  EXPECT_EQ(kValue2, value[0]);
+  EXPECT_EQ(Status::OK, kvs.Get(keys[2], &value));
+  EXPECT_EQ(kValue2, value);
 
-  EXPECT_EQ(kvs.KeyCount(), 1);
+  EXPECT_EQ(kvs.KeyCount(), 1u);
 }
 
 TEST_F(KeyValueStoreTest, BadCrc) {
@@ -734,13 +741,11 @@
   EXPECT_EQ(kvs_local_.Enable(), Status::OK);
   EXPECT_TRUE(kvs_local_.IsEnabled());
 
-  EXPECT_EQ(kvs_local_.Get(keys[0], buffer.data(), 1), Status::DATA_LOSS);
+  EXPECT_EQ(kvs_local_.Get(keys[0], span(buffer.data(), 1)), Status::DATA_LOSS);
 
   // Value with correct CRC should still be available.
   uint32_t test2 = 0;
-  ASSERT_EQ(Status::OK,
-            kvs_local_.Get(
-                keys[1], reinterpret_cast<uint8_t*>(&test2), sizeof(test2)));
+  ASSERT_EQ(Status::OK, kvs_local_.Get(keys[1], &test2));
   EXPECT_EQ(kTestPattern, test2);
 
   // Test rewriting over corrupted data.
@@ -775,8 +780,7 @@
     EXPECT_EQ(Status::OK, kvs_local_.Enable());
     uint32_t test2 = 0;
     ASSERT_EQ(Status::OK,
-              kvs_local_.Get(
-                  keys[1], reinterpret_cast<uint8_t*>(&test2), sizeof(test2)));
+              kvs_local_.Get(keys[1], as_writable_bytes(span(&test2, 1))));
     EXPECT_EQ(kTestPattern, test2);
   }
 }
@@ -792,11 +796,9 @@
   EXPECT_EQ(Status::OK, kvs_local_.Enable());
   // Write value
   const uint8_t kValue = 0xDA;
-  ASSERT_EQ(Status::OK, kvs_local_.Put(keys[0], &kValue, sizeof(kValue)));
+  ASSERT_EQ(Status::OK, kvs_local_.Put(keys[0], kValue));
   uint8_t value;
-  ASSERT_EQ(Status::OK,
-            kvs_local_.Get(
-                keys[0], reinterpret_cast<uint8_t*>(&value), sizeof(value)));
+  ASSERT_EQ(Status::OK, kvs_local_.Get(keys[0], &value));
 
   // Verify
   EXPECT_EQ(kValue, value);
@@ -811,17 +813,17 @@
 
   // Write value
   const uint8_t kValue = 0xDA;
-  ASSERT_EQ(Status::OK, kvs.Put(keys[0], &kValue, sizeof(kValue)));
+  ASSERT_EQ(Status::OK, kvs.Put(keys[0], kValue));
 
   ASSERT_EQ(Status::OK, kvs.Erase(keys[0]));
-  uint8_t value[1];
-  ASSERT_EQ(kvs.Get(keys[0], value, sizeof(value)), Status::NOT_FOUND);
+  uint8_t value;
+  ASSERT_EQ(kvs.Get(keys[0], &value), Status::NOT_FOUND);
 
   // Reset KVS, ensure captured at enable
   kvs.Disable();
   ASSERT_EQ(Status::OK, kvs.Enable());
 
-  ASSERT_EQ(kvs.Get(keys[0], value, sizeof(value)), Status::NOT_FOUND);
+  ASSERT_EQ(kvs.Get(keys[0], &value), Status::NOT_FOUND);
 }
 
 TEST_F(KeyValueStoreTest, TemplatedPutAndGet) {
@@ -992,10 +994,11 @@
 
   const char* kNewKey = "NewKey";
   constexpr size_t kValueLessThanChunkHeaderSize = 2;
-  constexpr uint8_t kTestPattern = 0xBA;
+  constexpr auto kTestPattern = byte{0xBA};
   new_keyvalue_size -= kValueLessThanChunkHeaderSize;
-  memset(buffer.data(), kTestPattern, new_keyvalue_size);
-  ASSERT_EQ(Status::OK, kvs.Put(kNewKey, buffer.data(), new_keyvalue_size));
+  std::memset(buffer.data(), static_cast<int>(kTestPattern), new_keyvalue_size);
+  ASSERT_EQ(Status::OK,
+            kvs.Put(kNewKey, span(buffer.data(), new_keyvalue_size)));
 
   // In failed corner case, adding new key is deceptively successful. It isn't
   // until KVS is disabled and reenabled that issue can be detected.
@@ -1003,7 +1006,8 @@
   ASSERT_EQ(Status::OK, kvs.Enable());
 
   // Might as well check that new key-value is what we expect it to be
-  ASSERT_EQ(Status::OK, kvs.Get(kNewKey, buffer.data(), new_keyvalue_size));
+  ASSERT_EQ(Status::OK,
+            kvs.Get(kNewKey, span(buffer.data(), new_keyvalue_size)));
   for (size_t i = 0; i < new_keyvalue_size; i++) {
     EXPECT_EQ(buffer[i], kTestPattern);
   }
@@ -1012,32 +1016,29 @@
 TEST_F(KeyValueStoreTest, GetValueSizeTests) {
   constexpr uint16_t kSizeOfValueToFill = 20U;
   constexpr uint8_t kKey0Pattern = 0xBA;
-  uint16_t value_size = 0;
   // Start off with disabled KVS
   kvs.Disable();
 
   // Try getting value when KVS is disabled, expect failure
-  EXPECT_EQ(kvs.GetValueSize(keys[0], &value_size),
-            Status::FAILED_PRECONDITION);
+  EXPECT_EQ(kvs.GetValueSize(keys[0]).status(), Status::FAILED_PRECONDITION);
 
   // Reset KVS
   test_partition.Erase(0, test_partition.GetSectorCount());
   ASSERT_EQ(Status::OK, kvs.Enable());
 
   // Try some case that are expected to fail
-  ASSERT_EQ(kvs.GetValueSize(keys[0], &value_size), Status::NOT_FOUND);
-  ASSERT_EQ(kvs.GetValueSize(nullptr, &value_size), Status::INVALID_ARGUMENT);
-  ASSERT_EQ(kvs.GetValueSize(keys[0], nullptr), Status::INVALID_ARGUMENT);
+  ASSERT_EQ(kvs.GetValueSize(keys[0]).status(), Status::NOT_FOUND);
+  ASSERT_EQ(kvs.GetValueSize("").status(), Status::INVALID_ARGUMENT);
 
   // Add key[0] and test we get the right value size for it.
-  memset(buffer.data(), kKey0Pattern, kSizeOfValueToFill);
-  ASSERT_EQ(Status::OK, kvs.Put(keys[0], buffer.data(), kSizeOfValueToFill));
-  ASSERT_EQ(Status::OK, kvs.GetValueSize(keys[0], &value_size));
-  ASSERT_EQ(value_size, kSizeOfValueToFill);
+  std::memset(buffer.data(), kKey0Pattern, kSizeOfValueToFill);
+  ASSERT_EQ(Status::OK,
+            kvs.Put(keys[0], span(buffer.data(), kSizeOfValueToFill)));
+  ASSERT_EQ(kSizeOfValueToFill, kvs.GetValueSize(keys[0]).size());
 
   // Verify after erase key is not found
   ASSERT_EQ(Status::OK, kvs.Erase(keys[0]));
-  ASSERT_EQ(kvs.GetValueSize(keys[0], &value_size), Status::NOT_FOUND);
+  ASSERT_EQ(kvs.GetValueSize(keys[0]).status(), Status::NOT_FOUND);
 }
 
 TEST_F(KeyValueStoreTest, CanFitEntryTests) {
@@ -1085,8 +1086,8 @@
 
   // Read it back and check it is correct
   char value[3];
-  ASSERT_EQ(Status::OK, kvs.Get(kKey, value, sizeof(value)));
-  ASSERT_EQ(memcmp(value, kValue2, sizeof(value)), 0);
+  ASSERT_EQ(Status::OK, kvs.Get(kKey, &value));
+  ASSERT_EQ(std::memcmp(value, kValue2, sizeof(value)), 0);
 }
 
 TEST_F(KeyValueStoreTest, CallingEraseTwice) {
@@ -1097,7 +1098,7 @@
   ASSERT_EQ(Status::OK, kvs.Enable());
 
   const uint8_t kValue = 0xDA;
-  ASSERT_EQ(Status::OK, kvs.Put(keys[0], &kValue, sizeof(kValue)));
+  ASSERT_EQ(Status::OK, kvs.Put(keys[0], kValue));
   ASSERT_EQ(Status::OK, kvs.Erase(keys[0]));
   uint16_t crc = CalcTestPartitionCrc();
   EXPECT_EQ(kvs.Erase(keys[0]), Status::NOT_FOUND);
@@ -1430,18 +1431,18 @@
 
   const uint8_t kValue1 = 0xDA;
   const uint8_t kValue2 = 0x12;
-  uint8_t value[1];
-  ASSERT_EQ(Status::OK, large_kvs.Put(keys[0], &kValue1, sizeof(kValue1)));
-  EXPECT_EQ(large_kvs.KeyCount(), 1);
+  uint8_t value;
+  ASSERT_EQ(Status::OK, large_kvs.Put(keys[0], kValue1));
+  EXPECT_EQ(large_kvs.KeyCount(), 1u);
   ASSERT_EQ(Status::OK, large_kvs.Erase(keys[0]));
-  EXPECT_EQ(large_kvs.Get(keys[0], value, sizeof(value)), Status::NOT_FOUND);
-  ASSERT_EQ(Status::OK, large_kvs.Put(keys[1], &kValue1, sizeof(kValue1)));
-  ASSERT_EQ(Status::OK, large_kvs.Put(keys[2], &kValue2, sizeof(kValue2)));
+  EXPECT_EQ(large_kvs.Get(keys[0], &value), Status::NOT_FOUND);
+  ASSERT_EQ(Status::OK, large_kvs.Put(keys[1], kValue1));
+  ASSERT_EQ(Status::OK, large_kvs.Put(keys[2], kValue2));
   ASSERT_EQ(Status::OK, large_kvs.Erase(keys[1]));
-  EXPECT_EQ(Status::OK, large_kvs.Get(keys[2], value, sizeof(value)));
-  EXPECT_EQ(kValue2, value[0]);
+  EXPECT_EQ(Status::OK, large_kvs.Get(keys[2], &value));
+  EXPECT_EQ(kValue2, value);
   ASSERT_EQ(large_kvs.Get(keys[1], &value), Status::NOT_FOUND);
-  EXPECT_EQ(large_kvs.KeyCount(), 1);
+  EXPECT_EQ(large_kvs.KeyCount(), 1u);
 }
 #endif  // USE_MEMORY_BUFFER
 
diff --git a/pw_kvs/public/pw_kvs/assert.h b/pw_kvs/public/pw_kvs/assert.h
index 36f232b..cc20a61 100644
--- a/pw_kvs/public/pw_kvs/assert.h
+++ b/pw_kvs/public/pw_kvs/assert.h
@@ -39,7 +39,7 @@
   static_assert(!std::is_null_pointer<T>(),
                 "CHECK_NOTNULL statements cannot be passed nullptr");
   if (value == nullptr) {
-    std::exit(1);
+    // std::exit(1);
   }
   return std::forward<T>(value);
 }
diff --git a/pw_kvs/public/pw_kvs/flash.h b/pw_kvs/public/pw_kvs/flash.h
index 6eecea7..819f8e2 100644
--- a/pw_kvs/public/pw_kvs/flash.h
+++ b/pw_kvs/public/pw_kvs/flash.h
@@ -21,12 +21,12 @@
 // bytes with 0.
 Status PaddedWrite(FlashPartition* partition,
                    FlashPartition::Address address,
-                   const uint8_t* buffer,
+                   const void* buffer,
                    uint16_t size);
 
 // Read into a buffer when size is not guaranteed to be aligned.
 Status UnalignedRead(FlashPartition* partition,
-                     uint8_t* buffer,
+                     void* buffer,
                      FlashPartition::Address address,
                      uint16_t size);
 
diff --git a/pw_kvs/public/pw_kvs/key_value_store.h b/pw_kvs/public/pw_kvs/key_value_store.h
index d9bb34f..2faa512 100644
--- a/pw_kvs/public/pw_kvs/key_value_store.h
+++ b/pw_kvs/public/pw_kvs/key_value_store.h
@@ -13,12 +13,18 @@
 // the License.
 #pragma once
 
-#include <algorithm>
+#include <cstddef>
+#include <cstdint>
+#include <limits>
+#include <string_view>
 #include <type_traits>
 
 #include "pw_kvs/flash_memory.h"
+#include "pw_span/span.h"
 #include "pw_status/status.h"
+#include "pw_status/status_with_size.h"
 
+namespace pw::kvs {
 namespace cfg {
 
 // KVS requires a temporary buffer for some operations, this config allows
@@ -41,20 +47,37 @@
 
 }  // namespace cfg
 
-namespace pw::kvs {
+namespace internal {
 
-// This object is very large and should not be placed on the stack.
+// Traits class to detect if the type is a span. std::is_same is insufficient
+// because span is a class template. This is used to ensure that the correct
+// overload of the Put function is selected.
+template <typename>
+struct IsSpan : std::false_type {};
+
+template <typename T, size_t kExtent>
+struct IsSpan<span<T, kExtent>> : std::true_type {};
+
+}  // namespace internal
+
+// This object is very large (can be over 1500 B, depending on configuration)
+// and should not typically be placed on the stack.
 class KeyValueStore {
  public:
   constexpr KeyValueStore(FlashPartition* partition) : partition_(*partition) {}
 
+  KeyValueStore(const KeyValueStore&) = delete;
+  KeyValueStore& operator=(const KeyValueStore&) = delete;
+
   // Enable the KVS, scans the sectors of the partition for any current KVS
   // data. Erases and initializes any sectors which are not initialized.
   // Checks the CRC of all data in the KVS; on failure the corrupted data is
   // lost and Enable will return Status::DATA_LOSS, but the KVS will still load
   // other data and will be enabled.
   Status Enable();
+
   bool IsEnabled() const { return enabled_; }
+
   void Disable() {
     if (enabled_ == false) {
       return;
@@ -67,7 +90,7 @@
   // key is a null terminated c string.
   // Returns OK if erased
   //         NOT_FOUND if key was not found.
-  Status Erase(const char* key);
+  Status Erase(const std::string_view& key);
 
   // Copy the first size bytes of key's value into value buffer.
   // key is a null terminated c string. Size is the amount of bytes to read,
@@ -76,7 +99,9 @@
   // Returns OK if successful
   //         NOT_FOUND if key was not found
   //         DATA_LOSS if the value failed crc check
-  Status Get(const char* key, void* value, uint16_t size, uint16_t offset = 0);
+  Status Get(const std::string_view& key,
+             const span<std::byte>& value,
+             uint16_t offset = 0);
 
   // Interpret the key's value as the given type and return it.
   // Returns OK if successful
@@ -84,37 +109,35 @@
   //         DATA_LOSS if the value failed crc check
   //         INVALID_ARGUMENT if size of value != sizeof(T)
   template <typename T>
-  Status Get(const char* key, T* value) {
+  Status Get(const std::string_view& key, T* value) {
     static_assert(std::is_trivially_copyable<T>(), "KVS values must copyable");
     static_assert(!std::is_pointer<T>(), "KVS values cannot be pointers");
 
-    uint16_t value_size = 0;
-    if (Status status = GetValueSize(key, &value_size)) {
-      return status;
+    StatusWithSize result = GetValueSize(key);
+    if (!result.ok()) {
+      return result.status();
     }
-    if (value_size != sizeof(T)) {
+    if (result.size() != sizeof(T)) {
       return Status::INVALID_ARGUMENT;
     }
-    return Get(key, value, sizeof(T));
+    return Get(key, as_writable_bytes(span(value, 1)));
   }
 
   // Writes the key value store to the partition. If the key already exists it
   // will be deleted before writing the new value.
-  // key is a null terminated c string.
+  //
   // Returns OK if successful
   //         RESOURCE_EXHAUSTED if there is not enough available space
-  Status Put(const char* key, const void* value, uint16_t size);
+  Status Put(const std::string_view& key, const span<const std::byte>& value);
 
-  // Writes the value to the partition. If the key already exists it will be
-  // deleted before writing the new value.
-  // key is a null terminated c string.
-  // Returns OK if successful
-  //         RESOURCE_EXHAUSTED if there is not enough available space
-  template <typename T>
-  Status Put(const char* key, const T& value) {
-    static_assert(std::is_trivially_copyable<T>(), "KVS values must copyable");
-    static_assert(!std::is_pointer<T>(), "KVS values cannot be pointers");
-    return Put(key, &value, sizeof(T));
+  // Alternate Put function that takes an object. The object must be trivially
+  // copyable and cannot be a pointer or span.
+  template <typename T,
+            typename = std::enable_if_t<std::is_trivially_copyable_v<T> &&
+                                        !std::is_pointer_v<T> &&
+                                        !internal::IsSpan<T>()>>
+  Status Put(const std::string_view& key, const T& value) {
+    return Put(key, as_bytes(span(&value, 1)));
   }
 
   // Gets the size of the value stored for provided key.
@@ -122,7 +145,7 @@
   //         INVALID_ARGUMENT if args are invalid.
   //         FAILED_PRECONDITION if KVS is not enabled.
   //         NOT_FOUND if key was not found.
-  Status GetValueSize(const char* key, uint16_t* value_size);
+  StatusWithSize GetValueSize(const std::string_view& key);
 
   // Tests if the proposed key value entry can be stored in the KVS.
   bool CanFitEntry(uint16_t key_len, uint16_t value_len) {
@@ -150,22 +173,20 @@
 
   // For debugging, logging, and testing. (Don't use in regular code)
   // Note: a key_index is not valid after an element is erased or updated.
-  uint8_t KeyCount() const;
-  const char* GetKey(uint8_t idx) const;
-  uint16_t GetValueSize(uint8_t idx) const;
+  size_t KeyCount() const;
+  std::string_view GetKey(size_t idx) const;
+  size_t GetValueSize(size_t idx) const;
   size_t GetMaxKeys() const { return kListCapacityMax; }
-  bool HasEmptySector() const { return HaveEmptySectorImpl(); }
+  bool HasEmptySector() const { return HasEmptySectorImpl(kSectorInvalid); }
 
   static constexpr size_t kHeaderSize = 8;  // Sector and KVS Header size
   static constexpr uint16_t MaxValueLength() { return kChunkValueLengthMax; }
-  KeyValueStore(const KeyValueStore&) = delete;
-  KeyValueStore& operator=(const KeyValueStore&) = delete;
 
  private:
   using KeyIndex = uint8_t;
   using SectorIndex = uint32_t;
 
-  static constexpr uint16_t kVersion = 4;
+  static constexpr uint16_t kVersion = 1;
   static constexpr KeyIndex kListCapacityMax = cfg::kKvsMaxKeyCount;
   static constexpr SectorIndex kSectorCountMax = cfg::kKvsMaxSectorCount;
 
@@ -187,7 +208,6 @@
   static constexpr FlashPartition::Address kAddressInvalid = 0xFFFFFFFFul;
   static constexpr uint64_t kSectorCleanNotPending = 0xFFFFFFFFFFFFFFFFull;
 
-  // TODO: Use BitPacker if/when have more flags.
   static constexpr uint16_t kFlagsIsErasedMask = 0x0001;
   static constexpr uint16_t kMaxAlignmentBytes = 128;
 
@@ -197,7 +217,7 @@
     uint16_t version;
     uint16_t alignment_bytes;  // alignment used for each chunk in this sector.
     uint16_t padding;          // padding to support uint64_t alignment.
-  } __attribute__((__packed__));
+  };
   static_assert(sizeof(KvsSectorHeaderMeta) == kHeaderSize,
                 "Invalid KvsSectorHeaderMeta size");
 
@@ -230,16 +250,26 @@
     // That way we can work out the length of the value as:
     // (chunk length - key length - size of chunk header).
     uint16_t chunk_len : kChunkHeaderChunkFieldNumBits;
-  } __attribute__((__packed__));
+  };
   static_assert(sizeof(KvsHeader) == kHeaderSize, "Invalid KvsHeader size");
 
   struct KeyMap {
-    char key[kChunkKeyLengthMax + 1];  // + 1 for null terminator
+    std::string_view key() const { return {key_buffer, key_length}; }
+
     FlashPartition::Address address;
-    uint16_t chunk_len;
+    char key_buffer[kChunkKeyLengthMax + 1];  // +1 for null terminator
+    uint8_t key_length;
     bool is_erased;
+    uint16_t chunk_len;
+
+    static_assert(kChunkKeyLengthMax <=
+                  std::numeric_limits<decltype(key_length)>::max());
   };
 
+  static constexpr bool InvalidKey(const std::string_view& key) {
+    return key.empty() || key.size() > kChunkKeyLengthMax;
+  }
+
   // NOTE: All public APIs handle the locking, the internal methods assume the
   // lock has already been acquired.
 
@@ -252,54 +282,48 @@
   }
 
   // Returns kAddressInvalid if no space is found, otherwise the address.
-  FlashPartition::Address FindSpace(uint16_t requested_size) const;
+  FlashPartition::Address FindSpace(size_t requested_size) const;
 
   // Attempts to rewrite a key's value by appending the new value to the same
   // sector. If the sector is full the value is written to another sector, and
   // the sector is marked for cleaning.
   // Returns RESOURCE_EXHAUSTED if no space is available, OK otherwise.
   Status RewriteValue(KeyIndex key_index,
-                      const uint8_t* value,
-                      uint16_t size,
+                      const span<const std::byte>& value,
                       bool is_erased = false);
 
   bool ValueMatches(KeyIndex key_index,
-                    const uint8_t* value,
-                    uint16_t size,
+                    const span<const std::byte>& value,
                     bool is_erased);
 
   // ResetSector erases the sector and writes the sector header.
   Status ResetSector(SectorIndex sector_index);
   Status WriteKeyValue(FlashPartition::Address address,
-                       const char* key,
-                       const uint8_t* value,
-                       uint16_t size,
+                       const std::string_view& key,
+                       const span<const std::byte>& value,
                        bool is_erased = false);
   uint32_t SectorSpaceRemaining(SectorIndex sector_index) const;
 
   // Returns idx if key is found, otherwise kListCapacityMax.
-  KeyIndex FindKeyInMap(const char* key) const;
-  bool IsKeyInMap(const char* key) const {
+  KeyIndex FindKeyInMap(const std::string_view& key) const;
+  bool IsKeyInMap(const std::string_view& key) const {
     return FindKeyInMap(key) != kListCapacityMax;
   }
 
   void RemoveFromMap(KeyIndex key_index);
-  Status AppendToMap(const char* key,
+  Status AppendToMap(const std::string_view& key,
                      FlashPartition::Address address,
-                     uint16_t chunk_len,
+                     size_t chunk_len,
                      bool is_erased = false);
   void UpdateMap(KeyIndex key_index,
                  FlashPartition::Address address,
                  uint16_t chunk_len,
                  bool is_erased = false);
-  uint16_t CalculateCrc(const char* key,
-                        uint16_t key_size,
-                        const uint8_t* value,
-                        uint16_t value_size) const;
+  uint16_t CalculateCrc(const std::string_view& key,
+                        const span<const std::byte>& value) const;
 
   // Calculates the CRC by reading the value from flash in chunks.
-  Status CalculateCrcFromValueAddress(const char* key,
-                                      uint16_t key_size,
+  Status CalculateCrcFromValueAddress(const std::string_view& key,
                                       FlashPartition::Address value_address,
                                       uint16_t value_size,
                                       uint16_t* crc_ret);
@@ -326,9 +350,9 @@
                    uint16_t size);
 
   // Size of a chunk including header, key, value, and alignment padding.
-  uint16_t ChunkSize(uint16_t key_len, uint16_t chunk_len) const {
+  size_t ChunkSize(size_t key_length, size_t value_length) const {
     return RoundUpForAlignment(sizeof(KvsHeader)) +
-           RoundUpForAlignment(key_len) + RoundUpForAlignment(chunk_len);
+           RoundUpForAlignment(key_length) + RoundUpForAlignment(value_length);
   }
 
   // Sectors should be cleaned when full, every valid (Most recent, not erased)
@@ -353,7 +377,7 @@
            RoundUpForAlignment(sizeof(KvsSectorHeaderCleaning));
   }
 
-  bool HaveEmptySectorImpl(SectorIndex skip_sector = kSectorInvalid) const {
+  bool HasEmptySectorImpl(SectorIndex skip_sector) const {
     for (SectorIndex i = 0; i < SectorCount(); i++) {
       if (i != skip_sector &&
           sector_space_remaining_[i] == SectorSpaceAvailableWhenEmpty()) {
@@ -366,7 +390,7 @@
   bool IsInLastFreeSector(FlashPartition::Address address) {
     return sector_space_remaining_[AddressToSectorIndex(address)] ==
                SectorSpaceAvailableWhenEmpty() &&
-           !HaveEmptySectorImpl(AddressToSectorIndex(address));
+           !HasEmptySectorImpl(AddressToSectorIndex(address));
   }
 
   FlashPartition& partition_;
@@ -381,9 +405,8 @@
   KeyMap key_map_[kListCapacityMax] = {};
   KeyIndex map_size_ = 0;
 
-  // +1 for nul-terminator since keys are stored as Length + Value and no nul
-  // termination but we are using them as nul-terminated strings through
-  // loading-up and comparing the keys.
+  // Add +1 for a null terminator. The keys are used as string_views, but the
+  // null-terminator provides additional safetly.
   char temp_key_buffer_[kChunkKeyLengthMax + 1u] = {0};
   uint8_t temp_buffer_[cfg::kKvsBufferSize] = {0};
 };