pw_kvs: Add to build; get tests passing
Change-Id: Idfd357beaf60b94ebaf9b4b1f8cd1b3d712e9b10
diff --git a/pw_kvs/public/pw_kvs/assert.h b/pw_kvs/public/pw_kvs/assert.h
new file mode 100644
index 0000000..36f232b
--- /dev/null
+++ b/pw_kvs/public/pw_kvs/assert.h
@@ -0,0 +1,69 @@
+// Copyright 2020 The Pigweed Authors
+//
+// Licensed under the Apache License, Version 2.0 (the "License"); you may not
+// use this file except in compliance with the License. You may obtain a copy of
+// the License at
+//
+// https://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+// WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+// License for the specific language governing permissions and limitations under
+// the License.
+#pragma once
+
+#include <algorithm>
+#include <type_traits>
+
+// Compares the provided value to nullptr and returns it. This is intended to be
+// used as part of another statement.
+#define CHECK_NOTNULL(value) \
+ ::pw::log::CheckNotNull("", __LINE__, #value " != nullptr", value)
+
+// In release builds, DCHECK_NOTNULL simply passes along the value.
+// DCHECK_NOTNULL must not be used as a standalone expression, since the result
+// would be unused on release builds. Use DCHECK_NE instead.
+//#define DCHECK_NOTNULL(value) value
+
+#define DCHECK_NOTNULL(value) \
+ ::pw::log::DCheckNotNull("", __LINE__, #value " != nullptr", value)
+
+namespace pw::log {
+
+template <typename T>
+constexpr T CheckNotNull(const char* /* file */,
+ unsigned /* line */,
+ const char* /* message */,
+ T&& value) {
+ static_assert(!std::is_null_pointer<T>(),
+ "CHECK_NOTNULL statements cannot be passed nullptr");
+ if (value == nullptr) {
+ std::exit(1);
+ }
+ return std::forward<T>(value);
+}
+
+// DCHECK_NOTNULL cannot be used in standalone expressions, so add a
+// [[nodiscard]] attribute to prevent this in debug builds. Standalone
+// DCHECK_NOTNULL statements in release builds trigger an unused-value warning.
+template <typename T>
+[[nodiscard]] constexpr T DCheckNotNull(const char* file,
+ unsigned line,
+ const char* message,
+ T&& value) {
+ return CheckNotNull<T>(file, line, message, std::forward<T>(value));
+}
+
+} // namespace pw::log
+
+// Assert stubs
+#define DCHECK CHECK
+#define DCHECK_EQ CHECK_EQ
+
+#define CHECK(...)
+#define CHECK_EQ(...)
+#define CHECK_GE(...)
+#define CHECK_GT(...)
+#define CHECK_LE(...)
+#define CHECK_LT(...)
diff --git a/pw_kvs/public/pw_kvs/flash.h b/pw_kvs/public/pw_kvs/flash.h
index 5cb80cd..6eecea7 100644
--- a/pw_kvs/public/pw_kvs/flash.h
+++ b/pw_kvs/public/pw_kvs/flash.h
@@ -13,9 +13,9 @@
// the License.
#pragma once
-#include "pw_kvs/devices/flash_memory.h"
+#include "pw_kvs/flash_memory.h"
-namespace pw {
+namespace pw::kvs {
// Writes a buffer which is not guaranteed to be aligned, pads remaining
// bytes with 0.
@@ -30,4 +30,4 @@
FlashPartition::Address address,
uint16_t size);
-} // namespace pw
+} // namespace pw::kvs
diff --git a/pw_kvs/public/pw_kvs/flash_memory.h b/pw_kvs/public/pw_kvs/flash_memory.h
index 707f128..d62f794 100644
--- a/pw_kvs/public/pw_kvs/flash_memory.h
+++ b/pw_kvs/public/pw_kvs/flash_memory.h
@@ -14,14 +14,16 @@
#pragma once
#include <algorithm>
+#include <cinttypes>
+#include <cstring>
#include "pw_kvs/assert.h"
-#include "pw_kvs/logging.h"
-#include "pw_kvs/peripherals/partition_table_entry.h"
-#include "pw_kvs/status.h"
-#include "pw_kvs/status_macros.h"
+#include "pw_kvs/partition_table_entry.h"
+#include "pw_log/log.h"
+#include "pw_status/status.h"
-namespace pw {
+namespace pw::kvs {
+
class FlashMemory {
public:
// The flash address is in the range of: 0 to FlashSize.
@@ -39,6 +41,8 @@
sector_start_(sector_start),
erased_memory_content_(erased_memory_content) {}
+ virtual ~FlashMemory() = default;
+
virtual Status Enable() = 0;
virtual Status Disable() = 0;
virtual bool IsEnabled() const = 0;
@@ -72,9 +76,7 @@
// Convert an Address to an MCU pointer, this can be used for memory
// mapped reads. Return NULL if the memory is not memory mapped.
- virtual uint8_t* FlashAddressToMcuAddress(Address address) const {
- return nullptr;
- }
+ virtual uint8_t* FlashAddressToMcuAddress(Address) const { return nullptr; }
// GetStartSector() is useful for FlashMemory instances where the
// sector start is not 0. (ex.: cases where there are portions of flash
@@ -137,9 +139,7 @@
bool IsEnabled() const override { return flash_.IsEnabled(); }
Status SelfTest() override { return flash_.SelfTest(); }
- Status Erase(Address flash_address, uint32_t num_sectors) override {
- return Status::UNIMPLEMENTED;
- }
+ Status Erase(Address, uint32_t) override { return Status::UNIMPLEMENTED; }
Status Read(uint8_t* destination_ram_address,
Address source_flash_address,
@@ -185,6 +185,8 @@
entry.partition_start_sector_index + 1),
permission_(entry.partition_permission) {}
+ virtual ~FlashPartition() = default;
+
// Erase num_sectors starting at a given address. Blocking call.
// Address should be on a sector boundary.
// Returns: OK, on success.
@@ -193,9 +195,14 @@
// PERMISSION_DENIED, if partition is read only.
// UNKNOWN, on HAL error
virtual Status Erase(Address address, uint32_t num_sectors) {
- RETURN_STATUS_IF(permission_ == PartitionPermission::kReadOnly,
- Status::PERMISSION_DENIED);
- RETURN_IF_ERROR(CheckBounds(address, num_sectors * GetSectorSizeBytes()));
+ if (permission_ == PartitionPermission::kReadOnly) {
+ return Status::PERMISSION_DENIED;
+ }
+ if (Status status =
+ CheckBounds(address, num_sectors * GetSectorSizeBytes());
+ !status.ok()) {
+ return status;
+ }
return flash_.Erase(PartitionToFlashAddress(address), num_sectors);
}
@@ -207,7 +214,9 @@
virtual Status Read(uint8_t* destination_ram_address,
Address source_flash_address,
uint32_t len) {
- RETURN_IF_ERROR(CheckBounds(source_flash_address, len));
+ if (Status status = CheckBounds(source_flash_address, len); !status.ok()) {
+ return status;
+ }
return flash_.Read(destination_ram_address,
PartitionToFlashAddress(source_flash_address),
len);
@@ -222,9 +231,13 @@
virtual Status Write(Address destination_flash_address,
const uint8_t* source_ram_address,
uint32_t len) {
- RETURN_STATUS_IF(permission_ == PartitionPermission::kReadOnly,
- Status::PERMISSION_DENIED);
- RETURN_IF_ERROR(CheckBounds(destination_flash_address, len));
+ if (permission_ == PartitionPermission::kReadOnly) {
+ return Status::PERMISSION_DENIED;
+ }
+ if (Status status = CheckBounds(destination_flash_address, len);
+ !status.ok()) {
+ return status;
+ }
return flash_.Write(PartitionToFlashAddress(destination_flash_address),
source_ram_address,
len);
@@ -243,24 +256,31 @@
// function. Using 16 because it's the alignment of encrypted flash.
const uint8_t kMaxAlignment = 16;
// Relying on Read() to check address and len arguments.
- RETURN_STATUS_IF(!is_erased, Status::INVALID_ARGUMENT);
+ if (!is_erased) {
+ return Status::INVALID_ARGUMENT;
+ }
uint8_t alignment = GetAlignmentBytes();
- RETURN_STATUS_IF(alignment > kMaxAlignment, Status::INVALID_ARGUMENT);
- RETURN_STATUS_IF(kMaxAlignment % alignment, Status::INVALID_ARGUMENT);
- RETURN_STATUS_IF(len % alignment, Status::INVALID_ARGUMENT);
+ if (alignment > kMaxAlignment || kMaxAlignment % alignment ||
+ len % alignment) {
+ return Status::INVALID_ARGUMENT;
+ }
uint8_t buffer[kMaxAlignment];
uint8_t erased_pattern_buffer[kMaxAlignment];
size_t offset = 0;
- memset(erased_pattern_buffer,
- flash_.GetErasedMemoryContent(),
- sizeof(erased_pattern_buffer));
+ std::memset(erased_pattern_buffer,
+ flash_.GetErasedMemoryContent(),
+ sizeof(erased_pattern_buffer));
*is_erased = false;
while (len > 0) {
// Check earlier that len is aligned, no need to round up
uint16_t read_size = std::min(static_cast<uint32_t>(sizeof(buffer)), len);
- RETURN_IF_ERROR(Read(buffer, source_flash_address + offset, read_size));
- if (memcmp(buffer, erased_pattern_buffer, read_size)) {
+ if (Status status =
+ Read(buffer, source_flash_address + offset, read_size);
+ !status.ok()) {
+ return status;
+ }
+ if (std::memcmp(buffer, erased_pattern_buffer, read_size)) {
// Detected memory chunk is not entirely erased
return Status::OK;
}
@@ -301,8 +321,11 @@
protected:
Status CheckBounds(Address address, uint32_t len) const {
if (address + len > GetSizeBytes()) {
- LOG(ERROR) << "Attempted out-of-bound flash memory access (address:"
- << address << " length:" << len << ")";
+ PW_LOG_ERROR(
+ "Attempted out-of-bound flash memory access (address: %" PRIu32
+ " length: %zu)",
+ address,
+ len);
return Status::INVALID_ARGUMENT;
}
return Status::OK;
@@ -331,14 +354,20 @@
sector_count_(sector_count) {}
Status Erase(Address address, uint32_t num_sectors) override {
- RETURN_IF_ERROR(CheckBounds(address, num_sectors * GetSectorSizeBytes()));
+ if (Status status =
+ CheckBounds(address, num_sectors * GetSectorSizeBytes());
+ !status.ok()) {
+ return status;
+ }
return partition_->Erase(ParentAddress(address), num_sectors);
}
Status Read(uint8_t* destination_ram_address,
Address source_flash_address,
uint32_t len) override {
- RETURN_IF_ERROR(CheckBounds(source_flash_address, len));
+ if (Status status = CheckBounds(source_flash_address, len); !status.ok()) {
+ return status;
+ }
return partition_->Read(
destination_ram_address, ParentAddress(source_flash_address), len);
}
@@ -346,7 +375,10 @@
Status Write(Address destination_flash_address,
const uint8_t* source_ram_address,
uint32_t len) override {
- RETURN_IF_ERROR(CheckBounds(destination_flash_address, len));
+ if (Status status = CheckBounds(destination_flash_address, len);
+ !status.ok()) {
+ return status;
+ }
return partition_->Write(
ParentAddress(destination_flash_address), source_ram_address, len);
}
@@ -354,7 +386,9 @@
Status IsChunkErased(Address source_flash_address,
uint32_t len,
bool* is_erased) override {
- RETURN_IF_ERROR(CheckBounds(source_flash_address, len));
+ if (Status status = CheckBounds(source_flash_address, len); !status.ok()) {
+ return status;
+ }
return partition_->IsChunkErased(
ParentAddress(source_flash_address), len, is_erased);
}
@@ -374,4 +408,4 @@
const uint32_t sector_count_;
};
-} // namespace pw
+} // namespace pw::kvs
diff --git a/pw_kvs/public/pw_kvs/in_memory_fake_flash.h b/pw_kvs/public/pw_kvs/in_memory_fake_flash.h
index a67c770..d2ea67c 100644
--- a/pw_kvs/public/pw_kvs/in_memory_fake_flash.h
+++ b/pw_kvs/public/pw_kvs/in_memory_fake_flash.h
@@ -15,11 +15,10 @@
#include <array>
-#include "pw_kvs/devices/flash_memory.h"
-#include "pw_kvs/status.h"
-#include "pw_kvs/status_macros.h"
+#include "pw_kvs/flash_memory.h"
+#include "pw_status/status.h"
-namespace pw {
+namespace pw::kvs {
// This creates a buffer which mimics the behaviour of flash (requires erase,
// before write, checks alignments, and is addressed in sectors).
@@ -40,12 +39,15 @@
// INVALID_ARGUMENT, if address or sector count is invalid.
// UNKNOWN, on HAL error
Status Erase(Address addr, uint32_t num_sectors) override {
- RETURN_STATUS_IF(addr % GetSectorSizeBytes() != 0,
- Status::INVALID_ARGUMENT);
- RETURN_STATUS_IF(
- addr / GetSectorSizeBytes() + num_sectors > GetSectorCount(),
- Status::UNKNOWN);
- RETURN_STATUS_IF(addr % GetAlignmentBytes() != 0, Status::INVALID_ARGUMENT);
+ if (addr % GetSectorSizeBytes() != 0) {
+ return Status::INVALID_ARGUMENT;
+ }
+ if (addr / GetSectorSizeBytes() + num_sectors > GetSectorCount()) {
+ return Status::UNKNOWN;
+ }
+ if (addr % GetAlignmentBytes() != 0) {
+ return Status::INVALID_ARGUMENT;
+ }
memset(&buffer_[addr], 0xFF, GetSectorSizeBytes() * num_sectors);
return Status::OK;
}
@@ -57,9 +59,9 @@
Status Read(uint8_t* dest_ram_addr,
Address source_flash_addr,
uint32_t len) override {
- RETURN_STATUS_IF(
- (source_flash_addr + len) >= GetSectorCount() * GetSizeBytes(),
- Status::INVALID_ARGUMENT);
+ if ((source_flash_addr + len) >= GetSectorCount() * GetSizeBytes()) {
+ return Status::INVALID_ARGUMENT;
+ }
memcpy(dest_ram_addr, &buffer_[source_flash_addr], len);
return Status::OK;
}
@@ -71,15 +73,16 @@
Status Write(Address dest_flash_addr,
const uint8_t* source_ram_addr,
uint32_t len) override {
- RETURN_STATUS_IF(
- (dest_flash_addr + len) >= GetSectorCount() * GetSizeBytes(),
- Status::INVALID_ARGUMENT);
- RETURN_STATUS_IF(dest_flash_addr % GetAlignmentBytes() != 0,
- Status::INVALID_ARGUMENT);
- RETURN_STATUS_IF(len % GetAlignmentBytes() != 0, Status::INVALID_ARGUMENT);
+ if ((dest_flash_addr + len) >= GetSectorCount() * GetSizeBytes() ||
+ dest_flash_addr % GetAlignmentBytes() != 0 ||
+ len % GetAlignmentBytes() != 0) {
+ return Status::INVALID_ARGUMENT;
+ }
// Check in erased state
for (unsigned i = 0; i < len; i++) {
- RETURN_STATUS_IF(buffer_[dest_flash_addr + i] != 0xFF, Status::UNKNOWN);
+ if (buffer_[dest_flash_addr + i] != 0xFF) {
+ return Status::UNKNOWN;
+ }
}
memcpy(&buffer_[dest_flash_addr], source_ram_addr, len);
return Status::OK;
@@ -89,4 +92,4 @@
std::array<uint8_t, kSectorCount * kSectorSize> buffer_;
};
-} // namespace pw
+} // namespace pw::kvs
diff --git a/pw_kvs/public/pw_kvs/key_value_store.h b/pw_kvs/public/pw_kvs/key_value_store.h
index dca6507..d9bb34f 100644
--- a/pw_kvs/public/pw_kvs/key_value_store.h
+++ b/pw_kvs/public/pw_kvs/key_value_store.h
@@ -16,17 +16,37 @@
#include <algorithm>
#include <type_traits>
-#include "pw_kvs/devices/flash_memory.h"
-#include "pw_kvs/logging.h"
-#include "pw_kvs/os/mutex.h"
-#include "pw_kvs/status.h"
+#include "pw_kvs/flash_memory.h"
+#include "pw_status/status.h"
-namespace pw {
+namespace cfg {
+
+// KVS requires a temporary buffer for some operations, this config allows
+// tuning the buffer size. This is a trade-off between a value which is large
+// and therefore requires more RAM, or having a value which is small which will
+// result in some operations taking longer, as the operations are broken into
+// smaller chunks.
+// NOTE: This value can not be smaller then the flash alignment, and it will
+// round the size down to be a multiple of the flash alignment for all
+// operations.
+inline constexpr size_t kKvsBufferSize = 64;
+
+// This represents the maximum amount of keys which can be in the KVS at any
+// given time.
+inline constexpr uint8_t kKvsMaxKeyCount = 50;
+
+// This is the maximum amount of sectors the KVS can operate on, an invalid
+// value will cause an error during enable.
+inline constexpr uint32_t kKvsMaxSectorCount = 20;
+
+} // namespace cfg
+
+namespace pw::kvs {
// This object is very large and should not be placed on the stack.
class KeyValueStore {
public:
- KeyValueStore(FlashPartition* partition) : partition_(*partition) {}
+ constexpr KeyValueStore(FlashPartition* partition) : partition_(*partition) {}
// Enable the KVS, scans the sectors of the partition for any current KVS
// data. Erases and initializes any sectors which are not initialized.
@@ -39,7 +59,7 @@
if (enabled_ == false) {
return;
}
- os::MutexLock lock(&lock_);
+ // TODO: LOCK MUTEX
enabled_ = false;
}
@@ -67,9 +87,14 @@
Status Get(const char* 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;
- RETURN_IF_ERROR(GetValueSize(key, &value_size));
- RETURN_STATUS_IF(value_size != sizeof(T), Status::INVALID_ARGUMENT);
+ if (Status status = GetValueSize(key, &value_size)) {
+ return status;
+ }
+ if (value_size != sizeof(T)) {
+ return Status::INVALID_ARGUMENT;
+ }
return Get(key, value, sizeof(T));
}
@@ -107,11 +132,11 @@
// CleanAll cleans each sector which is currently marked for cleaning.
// Note: if any data is invalid/corrupt it could be lost.
Status CleanAll() {
- os::MutexLock lock(&lock_);
+ // TODO: LOCK MUTEX
return CleanAllInternal();
}
size_t PendingCleanCount() {
- os::MutexLock lock(&lock_);
+ // TODO: LOCK MUTEX
size_t ret = 0;
for (size_t i = 0; i < SectorCount(); i++) {
ret += sector_space_remaining_[i] == 0 ? 1 : 0;
@@ -345,7 +370,7 @@
}
FlashPartition& partition_;
- os::Mutex lock_;
+ // TODO: MUTEX
bool enabled_ = false;
uint8_t alignment_bytes_ = 0;
uint64_t next_sector_clean_order_ = 0;
@@ -353,7 +378,7 @@
// Free space available in each sector, set to 0 when clean is pending/active
uint32_t sector_space_remaining_[kSectorCountMax] = {0};
uint64_t sector_clean_order_[kSectorCountMax] = {kSectorCleanNotPending};
- KeyMap key_map_[kListCapacityMax] = {{{0}}};
+ KeyMap key_map_[kListCapacityMax] = {};
KeyIndex map_size_ = 0;
// +1 for nul-terminator since keys are stored as Length + Value and no nul
@@ -363,4 +388,4 @@
uint8_t temp_buffer_[cfg::kKvsBufferSize] = {0};
};
-} // namespace pw
+} // namespace pw::kvs
