commit cf680ab1b93965bc841ab408a49ef91b7ad5e303
author David Rogers <davidrogers@google.com> Wed Feb 12 23:28:32 2020 -0800
committer David Rogers <davidrogers@google.com> Thu Feb 13 11:28:16 2020 -0800
tree 86afbbb62ae819a4f8070dbf73bdf5c4b07c67b9
parent be744c19d6dd478791c4c3a2d1102c39879c2aef

pw_kvs: Fix return value for Get()

- Update KVS Get() to properly return the size of the value read when
verify_on_read is true. Previously only the status portion of the return was valid.
- Enable KVS test RepeatingValueWithOtherData.
- Add LogKeyDescriptor() helper and update LogSectors().

Change-Id: I5003fc58ed7d4342fd3b66155eda7ef8569e6aa6

pw_kvs/key_value_store.cc

diff --git a/pw_kvs/key_value_store.cc b/pw_kvs/key_value_store.cc
index 7ee058e..9949b88 100644
--- a/pw_kvs/key_value_store.cc
+++ b/pw_kvs/key_value_store.cc
@@ -261,9 +261,16 @@
 
   StatusWithSize result = ReadEntryValue(*key_descriptor, header, value_buffer);
   if (result.ok() && options_.verify_on_read) {
-    return header.VerifyChecksum(entry_header_format_.checksum,
-                                 key,
-                                 value_buffer.subspan(0, result.size()));
+    Status verify_result =
+        header.VerifyChecksum(entry_header_format_.checksum,
+                              key,
+                              value_buffer.subspan(0, result.size()));
+    if (!verify_result.ok()) {
+      memset(value_buffer.subspan(0, result.size()).data(), 0, result.size());
+      return verify_result;
+    }
+
+    return StatusWithSize(verify_result, result.size());
   }
   return result;
 }
@@ -846,7 +853,8 @@
   DBG("////////////////////// KEY VALUE STORE DUMP END /////////////////////");
 }
 
-void KeyValueStore::LogSectors(void) {
+void KeyValueStore::LogSectors() const {
+  DBG("Sector descriptors: count %zu", sectors_.size());
   for (auto& sector : sectors_) {
     DBG("  - Sector %zu: valid %hu, recoverable %zu, free %hu",
         SectorIndex(&sector),
@@ -856,6 +864,17 @@
   }
 }
 
+void KeyValueStore::LogKeyDescriptor() const {
+  DBG("Key descriptors: count %zu", key_descriptors_.size());
+  for (auto& key : key_descriptors_) {
+    DBG("  - Key: %s, hash %#zx, version %zu, address %#zx",
+        key.deleted() ? "Deleted" : "Valid",
+        static_cast<size_t>(key.key_hash),
+        static_cast<size_t>(key.key_version),
+        static_cast<size_t>(key.address));
+  }
+}
+
 void KeyValueStore::SectorDescriptor::RemoveValidBytes(size_t size) {
   // TODO: add safety check for valid_bytes > size.
   if (size > valid_bytes) {

pw_kvs/key_value_store_test.cc

diff --git a/pw_kvs/key_value_store_test.cc b/pw_kvs/key_value_store_test.cc
index cc89c55..4f64343 100644
--- a/pw_kvs/key_value_store_test.cc
+++ b/pw_kvs/key_value_store_test.cc
@@ -745,7 +745,7 @@
   EXPECT_EQ(kvs_.size(), 1u);
 }
 
-TEST_F(EmptyInitializedKvs, DISABLED_RepeatingValueWithOtherData) {
+TEST_F(EmptyInitializedKvs, RepeatingValueWithOtherData) {
   std::byte set_buf[150];
   std::byte get_buf[sizeof(set_buf)];
 
@@ -759,7 +759,7 @@
   // that is already in env before each test
   for (size_t test_iteration = 0; test_iteration < sizeof(set_buf);
        test_iteration++) {
-    ASSERT_EQ(Status::OK, test_partition.Erase());
+    // TOD0: Add KVS erase
     // Add a constant unchanging entry so that the updates are not
     // the only entries in the env.  The size of this initial entry
     // we vary between no bytes to sizeof(set_buf).

pw_kvs/public/pw_kvs/key_value_store.h

diff --git a/pw_kvs/public/pw_kvs/key_value_store.h b/pw_kvs/public/pw_kvs/key_value_store.h
index bc6ff37..bb3d029 100644
--- a/pw_kvs/public/pw_kvs/key_value_store.h
+++ b/pw_kvs/public/pw_kvs/key_value_store.h
@@ -334,7 +334,7 @@
     return (sector.tail_free_bytes == partition_.sector_size_bytes());
   }
 
-  size_t RecoverableBytes(const SectorDescriptor& sector) {
+  size_t RecoverableBytes(const SectorDescriptor& sector) const {
     return partition_.sector_size_bytes() - sector.valid_bytes -
            sector.tail_free_bytes;
   }
@@ -355,7 +355,8 @@
     return &sectors_[index];
   }
 
-  void LogSectors(void);
+  void LogSectors() const;
+  void LogKeyDescriptor() const;
 
   Address NextWritableAddress(SectorDescriptor* sector) const {
     return SectorBaseAddress(sector) + partition_.sector_size_bytes() -