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};
};
