pw_kvs: Checksum whole entry, including padding
- Always pad entries to their alignment instead of the flash's
alignment, which may be smaller.
- Checksum entire entries, including padding. The checksum is calculated
with the checksum field set to 0.
Change-Id: Ie354bf3fb93f23354d2278e5b16bd7d448728d95
diff --git a/pw_kvs/alignment_test.cc b/pw_kvs/alignment_test.cc
index 232a8e3..d5bf620 100644
--- a/pw_kvs/alignment_test.cc
+++ b/pw_kvs/alignment_test.cc
@@ -85,6 +85,37 @@
EXPECT_EQ(15u, AlignDown(16, 15));
}
+TEST(Padding, Zero) {
+ EXPECT_EQ(0u, Padding(0, 1));
+ EXPECT_EQ(0u, Padding(0, 2));
+ EXPECT_EQ(0u, Padding(0, 15));
+}
+
+TEST(Padding, Aligned) {
+ for (size_t i = 1; i < 130; ++i) {
+ EXPECT_EQ(0u, Padding(i, i));
+ EXPECT_EQ(0u, Padding(2 * i, i));
+ EXPECT_EQ(0u, Padding(3 * i, i));
+ }
+}
+
+TEST(Padding, NonAligned_PowerOf2) {
+ EXPECT_EQ(31u, Padding(1, 32));
+ EXPECT_EQ(1u, Padding(31, 32));
+ EXPECT_EQ(31u, Padding(33, 32));
+ EXPECT_EQ(19u, Padding(45, 32));
+ EXPECT_EQ(1u, Padding(63, 32));
+ EXPECT_EQ(1u, Padding(127, 32));
+}
+
+TEST(Padding, NonAligned_NonPowerOf2) {
+ EXPECT_EQ(1u, Padding(1, 2));
+
+ EXPECT_EQ(14u, Padding(1, 15));
+ EXPECT_EQ(1u, Padding(14, 15));
+ EXPECT_EQ(14u, Padding(16, 15));
+}
+
constexpr std::string_view kData =
"123456789_123456789_123456789_123456789_123456789_" // 50
"123456789_123456789_123456789_123456789_123456789_"; // 100
diff --git a/pw_kvs/format.cc b/pw_kvs/format.cc
index 04c69de..cb1eaf5 100644
--- a/pw_kvs/format.cc
+++ b/pw_kvs/format.cc
@@ -14,6 +14,8 @@
#include "pw_kvs_private/format.h"
+#include <cinttypes>
+
#include "pw_kvs_private/macros.h"
#include "pw_log/log.h"
@@ -30,7 +32,7 @@
size_t alignment_bytes,
uint32_t key_version)
: header_{.magic = magic,
- .checksum = kNoChecksum,
+ .checksum = 0,
.alignment_units = alignment_bytes_to_units(alignment_bytes),
.key_length_bytes = static_cast<uint8_t>(key.size()),
.value_length_bytes = value_length_bytes,
@@ -50,7 +52,7 @@
string_view key,
span<const byte> value) const {
if (algorithm == nullptr) {
- return checksum() == kNoChecksum ? Status::OK : Status::DATA_LOSS;
+ return checksum() == 0 ? Status::OK : Status::DATA_LOSS;
}
CalculateChecksum(algorithm, key, value);
return algorithm->Verify(checksum_bytes());
@@ -59,28 +61,23 @@
Status Entry::VerifyChecksumInFlash(FlashPartition* partition,
FlashPartition::Address address,
ChecksumAlgorithm* algorithm) const {
- // Read the entire entry piece-by-piece into a small buffer.
- // TODO: This read may be unaligned. The partition can handle this, but
- // consider creating a API that skips the intermediate buffering.
- byte buffer[32];
+ // Read the entire entry piece-by-piece into a small buffer. If the entry is
+ // 32 B or less, only one read is required.
+ union {
+ EntryHeader header_to_verify;
+ byte buffer[sizeof(EntryHeader) * 2];
+ };
- // Read and compare the magic and checksum.
- TRY(partition->Read(address, checked_data_offset(), buffer));
- if (std::memcmp(this, buffer, checked_data_offset()) != 0) {
- static_assert(sizeof(unsigned) >= sizeof(uint32_t));
- unsigned actual_magic;
- std::memcpy(&actual_magic, &buffer[0], sizeof(uint32_t));
- unsigned actual_checksum;
- std::memcpy(&actual_checksum, &buffer[4], sizeof(uint32_t));
+ size_t bytes_to_read = size();
+ size_t read_size = std::min(sizeof(buffer), bytes_to_read);
- PW_LOG_ERROR(
- "Expected: magic=%08x, checksum=%08x; "
- "Actual: magic=%08x, checksum=%08x",
- unsigned(magic()),
- unsigned(checksum()),
- actual_magic,
- actual_checksum);
+ // Read the first chunk, which includes the header, and compare the checksum.
+ TRY(partition->Read(address, read_size, buffer));
+ if (header_to_verify.checksum != checksum()) {
+ PW_LOG_ERROR("Expected checksum %08" PRIx32 ", found %08" PRIx32,
+ checksum(),
+ header_to_verify.checksum);
return Status::DATA_LOSS;
}
@@ -88,20 +85,24 @@
return Status::OK;
}
+ // The checksum is calculated as if the header's checksum field were 0.
+ header_to_verify.checksum = 0;
+
algorithm->Reset();
- // Read and calculate the checksum of the remaining header, key, and value.
- address += checked_data_offset();
- size_t bytes_to_read = content_size() - checked_data_offset();
-
- while (bytes_to_read > 0u) {
- const size_t read_size = std::min(sizeof(buffer), bytes_to_read);
-
- TRY(partition->Read(address, read_size, buffer));
+ while (true) {
+ // Add the chunk in the buffer to the checksum.
algorithm->Update(buffer, read_size);
- address += read_size;
bytes_to_read -= read_size;
+ if (bytes_to_read == 0u) {
+ break;
+ }
+
+ // Read the next chunk into the buffer.
+ address += read_size;
+ read_size = std::min(sizeof(buffer), bytes_to_read);
+ TRY(partition->Read(address, read_size, buffer));
}
algorithm->Finish();
@@ -112,10 +113,25 @@
const string_view key,
span<const byte> value) const {
algorithm->Reset();
- algorithm->Update(reinterpret_cast<const byte*>(this) + checked_data_offset(),
- sizeof(*this) - checked_data_offset());
- algorithm->Update(as_bytes(span(key)));
- algorithm->Update(value);
+
+ {
+ EntryHeader header_for_checksum = header_;
+ header_for_checksum.checksum = 0;
+
+ algorithm->Update(&header_for_checksum, sizeof(header_for_checksum));
+ algorithm->Update(as_bytes(span(key)));
+ algorithm->Update(value);
+ }
+
+ // Update the checksum with 0s to pad the entry to its alignment boundary.
+ constexpr byte padding[kMinAlignmentBytes - 1] = {};
+ size_t padding_to_add = Padding(content_size(), alignment_bytes());
+
+ while (padding_to_add > 0u) {
+ const size_t chunk_size = std::min(padding_to_add, sizeof(padding));
+ algorithm->Update(padding, chunk_size);
+ padding_to_add -= chunk_size;
+ }
return algorithm->Finish();
}
diff --git a/pw_kvs/pw_kvs_private/format.h b/pw_kvs/pw_kvs_private/format.h
index bb68e78..0685703 100644
--- a/pw_kvs/pw_kvs_private/format.h
+++ b/pw_kvs/pw_kvs_private/format.h
@@ -143,7 +143,6 @@
return sizeof(EntryHeader) + key_length() + value_length();
}
- static constexpr uint32_t kNoChecksum = 0;
static constexpr uint32_t kKeyLengthMask = 0b111111;
static constexpr uint16_t kDeletedValueLength = 0xFFFF;
@@ -155,10 +154,6 @@
size_t alignment_bytes,
uint32_t key_version);
- static constexpr size_t checked_data_offset() {
- return offsetof(EntryHeader, alignment_units);
- }
-
span<const std::byte> checksum_bytes() const {
return as_bytes(span(&header_.checksum, 1));
}