commit 08d37d8295d341065e79cbc158ca39a13450ccbc
author Wyatt Hepler <hepler@google.com> Thu Feb 27 15:45:37 2020 -0800
committer CQ Bot Account <commit-bot@chromium.org> Fri Feb 28 04:49:30 2020 +0000
tree 8ad49a58681c8c63e68fe0540b7bdc0d93e235cb
parent a0ba54f64805b5b156ef7fe00c543a910e31bd7d

pw_kvs: Move KeyDescriptor from iterator to Item

- Use a KeyDescriptor* instead of an index for consistency with other
  KeyDescriptor uses.
- Move KeyDescriptor to the Item class, since Item is more closely
  associated with the KeyDescriptor.

Change-Id: I0677bb7a5d6ef36708b93da5e02ff5de09ece686

pw_kvs/key_value_store.cc

diff --git a/pw_kvs/key_value_store.cc b/pw_kvs/key_value_store.cc
index 91b80d9..0282380 100644
--- a/pw_kvs/key_value_store.cc
+++ b/pw_kvs/key_value_store.cc
@@ -393,32 +393,30 @@
       key_descriptor, KeyDescriptor::kDeleted, key, {});
 }
 
+void KeyValueStore::Item::ReadKey() {
+  key_buffer_.fill('\0');
+
+  Entry entry;
+  if (Entry::Read(kvs_.partition_, descriptor_->address(), &entry).ok()) {
+    entry.ReadKey(key_buffer_);
+  }
+}
+
 KeyValueStore::iterator& KeyValueStore::iterator::operator++() {
   // Skip to the next entry that is valid (not deleted).
-  while (++index_ < item_.kvs_.key_descriptors_.size() &&
-         descriptor().deleted()) {
+  while (++item_.descriptor_ != item_.kvs_.key_descriptors_.end() &&
+         item_.descriptor_->deleted()) {
   }
   return *this;
 }
 
-const KeyValueStore::Item& KeyValueStore::iterator::operator*() {
-  std::memset(item_.key_buffer_.data(), 0, item_.key_buffer_.size());
-
-  Entry entry;
-  if (Entry::Read(item_.kvs_.partition_, descriptor().address(), &entry).ok()) {
-    entry.ReadKey(item_.key_buffer_);
-  }
-
-  return item_;
-}
-
 KeyValueStore::iterator KeyValueStore::begin() const {
-  size_t i = 0;
+  const KeyDescriptor* descriptor = key_descriptors_.begin();
   // Skip over any deleted entries at the start of the descriptor list.
-  while (i < key_descriptors_.size() && key_descriptors_[i].deleted()) {
-    i += 1;
+  while (descriptor != key_descriptors_.end() && descriptor->deleted()) {
+    ++descriptor;
   }
-  return iterator(*this, i);
+  return iterator(*this, descriptor);
 }
 
 // TODO(hepler): The valid entry count could be tracked in the KVS to avoid the

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 666dd6f..b2b114f 100644
--- a/pw_kvs/public/pw_kvs/key_value_store.h
+++ b/pw_kvs/public/pw_kvs/key_value_store.h
@@ -177,9 +177,14 @@
    private:
     friend class iterator;
 
-    constexpr Item(const KeyValueStore& kvs) : kvs_(kvs), key_buffer_{} {}
+    constexpr Item(const KeyValueStore& kvs,
+                   const internal::KeyDescriptor* descriptor)
+        : kvs_(kvs), descriptor_(descriptor), key_buffer_{} {}
+
+    void ReadKey();
 
     const KeyValueStore& kvs_;
+    const internal::KeyDescriptor* descriptor_;
 
     // Buffer large enough for a null-terminated version of any valid key.
     std::array<char, internal::Entry::kMaxKeyLength + 1> key_buffer_;
@@ -192,7 +197,10 @@
     iterator& operator++(int) { return operator++(); }
 
     // Reads the entry's key from flash.
-    const Item& operator*();
+    const Item& operator*() {
+      item_.ReadKey();
+      return item_;
+    }
 
     const Item* operator->() {
       operator*();  // Read the key into the Item object.
@@ -200,31 +208,27 @@
     }
 
     constexpr bool operator==(const iterator& rhs) const {
-      return index_ == rhs.index_;
+      return item_.descriptor_ == rhs.item_.descriptor_;
     }
 
     constexpr bool operator!=(const iterator& rhs) const {
-      return index_ != rhs.index_;
+      return item_.descriptor_ != rhs.item_.descriptor_;
     }
 
    private:
     friend class KeyValueStore;
 
-    constexpr iterator(const KeyValueStore& kvs, size_t index)
-        : item_(kvs), index_(index) {}
-
-    const internal::KeyDescriptor& descriptor() const {
-      return item_.kvs_.key_descriptors_[index_];
-    }
+    constexpr iterator(const KeyValueStore& kvs,
+                       const internal::KeyDescriptor* descriptor)
+        : item_(kvs, descriptor) {}
 
     Item item_;
-    size_t index_;
   };
 
   using const_iterator = iterator;  // Standard alias for iterable types.
 
   iterator begin() const;
-  iterator end() const { return iterator(*this, key_descriptors_.size()); }
+  iterator end() const { return iterator(*this, key_descriptors_.end()); }
 
   // Returns the number of valid entries in the KeyValueStore.
   size_t size() const;