diff --git a/components/nvs_flash/include/nvs.h b/components/nvs_flash/include/nvs.h index 1d88217ee0..4fe4408609 100644 --- a/components/nvs_flash/include/nvs.h +++ b/components/nvs_flash/include/nvs.h @@ -54,6 +54,7 @@ typedef uint32_t nvs_handle; #define ESP_ERR_NVS_KEYS_NOT_INITIALIZED (ESP_ERR_NVS_BASE + 0x16) /*!< NVS key partition is uninitialized */ #define ESP_ERR_NVS_CORRUPT_KEY_PART (ESP_ERR_NVS_BASE + 0x17) /*!< NVS key partition is corrupt */ +#define ESP_ERR_NVS_CONTENT_DIFFERS (ESP_ERR_NVS_BASE + 0x18) /*!< Internal error; never returned by nvs API functions. NVS key is different in comparison */ #define NVS_DEFAULT_PART_NAME "nvs" /*!< Default partition name of the NVS partition in the partition table */ /** diff --git a/components/nvs_flash/src/nvs_page.cpp b/components/nvs_flash/src/nvs_page.cpp index cfe6987f8e..e1fb4dbe07 100644 --- a/components/nvs_flash/src/nvs_page.cpp +++ b/components/nvs_flash/src/nvs_page.cpp @@ -308,6 +308,58 @@ esp_err_t Page::readItem(uint8_t nsIndex, ItemType datatype, const char* key, vo return ESP_OK; } +esp_err_t Page::cmpItem(uint8_t nsIndex, ItemType datatype, const char* key, const void* data, size_t dataSize, uint8_t chunkIdx, VerOffset chunkStart) +{ + size_t index = 0; + Item item; + + if (mState == PageState::INVALID) { + return ESP_ERR_NVS_INVALID_STATE; + } + + esp_err_t rc = findItem(nsIndex, datatype, key, index, item, chunkIdx, chunkStart); + if (rc != ESP_OK) { + return rc; + } + + if (!isVariableLengthType(datatype)) { + if (dataSize != getAlignmentForType(datatype)) { + return ESP_ERR_NVS_TYPE_MISMATCH; + } + + if (memcmp(data, item.data, dataSize)) { + return ESP_ERR_NVS_CONTENT_DIFFERS; + } + return ESP_OK; + } + + if (dataSize < static_cast(item.varLength.dataSize)) { + return ESP_ERR_NVS_INVALID_LENGTH; + } + + const uint8_t* dst = reinterpret_cast(data); + size_t left = item.varLength.dataSize; + for (size_t i = index + 1; i < index + item.span; ++i) { + Item ditem; + rc = readEntry(i, ditem); + if (rc != ESP_OK) { + return rc; + } + size_t willCopy = ENTRY_SIZE; + willCopy = (left < willCopy)?left:willCopy; + if (memcmp(dst, ditem.rawData, willCopy)) { + return ESP_ERR_NVS_CONTENT_DIFFERS; + } + left -= willCopy; + dst += willCopy; + } + if (Item::calculateCrc32(reinterpret_cast(data), item.varLength.dataSize) != item.varLength.dataCrc32) { + return ESP_ERR_NVS_NOT_FOUND; + } + + return ESP_OK; +} + esp_err_t Page::eraseItem(uint8_t nsIndex, ItemType datatype, const char* key, uint8_t chunkIdx, VerOffset chunkStart) { size_t index = 0; diff --git a/components/nvs_flash/src/nvs_page.hpp b/components/nvs_flash/src/nvs_page.hpp index e98deb22f5..a491752406 100644 --- a/components/nvs_flash/src/nvs_page.hpp +++ b/components/nvs_flash/src/nvs_page.hpp @@ -94,6 +94,8 @@ public: esp_err_t readItem(uint8_t nsIndex, ItemType datatype, const char* key, void* data, size_t dataSize, uint8_t chunkIdx = CHUNK_ANY, VerOffset chunkStart = VerOffset::VER_ANY); + esp_err_t cmpItem(uint8_t nsIndex, ItemType datatype, const char* key, const void* data, size_t dataSize, uint8_t chunkIdx = CHUNK_ANY, VerOffset chunkStart = VerOffset::VER_ANY); + esp_err_t eraseItem(uint8_t nsIndex, ItemType datatype, const char* key, uint8_t chunkIdx = CHUNK_ANY, VerOffset chunkStart = VerOffset::VER_ANY); esp_err_t findItem(uint8_t nsIndex, ItemType datatype, const char* key, uint8_t chunkIdx = CHUNK_ANY, VerOffset chunkStart = VerOffset::VER_ANY); @@ -112,6 +114,12 @@ public: return readItem(nsIndex, itemTypeOf(value), key, &value, sizeof(value)); } + template + esp_err_t cmpItem(uint8_t nsIndex, const char* key, const T& value) + { + return cmpItem(nsIndex, itemTypeOf(value), key, &value, sizeof(value)); + } + template esp_err_t eraseItem(uint8_t nsIndex, const char* key) { diff --git a/components/nvs_flash/src/nvs_storage.cpp b/components/nvs_flash/src/nvs_storage.cpp index be92da0726..b6399d458c 100644 --- a/components/nvs_flash/src/nvs_storage.cpp +++ b/components/nvs_flash/src/nvs_storage.cpp @@ -269,6 +269,13 @@ esp_err_t Storage::writeItem(uint8_t nsIndex, ItemType datatype, const char* key VerOffset prevStart, nextStart; prevStart = nextStart = VerOffset::VER_0_OFFSET; if (findPage) { + // Do a sanity check that the item in question is actually being modified. + // If it isn't, it is cheaper to purposefully not write out new data. + // since it may invoke an erasure of flash. + if (cmpMultiPageBlob(nsIndex, key, data, dataSize) == ESP_OK) { + return ESP_OK; + } + if (findPage->state() == Page::PageState::UNINITIALIZED || findPage->state() == Page::PageState::INVALID) { ESP_ERROR_CHECK(findItem(nsIndex, datatype, key, findPage, item)); @@ -311,6 +318,13 @@ esp_err_t Storage::writeItem(uint8_t nsIndex, ItemType datatype, const char* key } } } else { + // Do a sanity check that the item in question is actually being modified. + // If it isn't, it is cheaper to purposefully not write out new data. + // since it may invoke an erasure of flash. + if (findPage != nullptr && + findPage->cmpItem(nsIndex, datatype, key, data, dataSize) == ESP_OK) { + return ESP_OK; + } Page& page = getCurrentPage(); err = page.writeItem(nsIndex, datatype, key, data, dataSize); @@ -443,6 +457,48 @@ esp_err_t Storage::readMultiPageBlob(uint8_t nsIndex, const char* key, void* dat return err; } +esp_err_t Storage::cmpMultiPageBlob(uint8_t nsIndex, const char* key, const void* data, size_t dataSize) +{ + Item item; + Page* findPage = nullptr; + + /* First read the blob index */ + auto err = findItem(nsIndex, ItemType::BLOB_IDX, key, findPage, item); + if (err != ESP_OK) { + return err; + } + + uint8_t chunkCount = item.blobIndex.chunkCount; + VerOffset chunkStart = item.blobIndex.chunkStart; + size_t readSize = item.blobIndex.dataSize; + size_t offset = 0; + + if (dataSize != readSize) { + return ESP_ERR_NVS_CONTENT_DIFFERS; + } + + /* Now read corresponding chunks */ + for (uint8_t chunkNum = 0; chunkNum < chunkCount; chunkNum++) { + err = findItem(nsIndex, ItemType::BLOB_DATA, key, findPage, item, static_cast (chunkStart) + chunkNum); + if (err != ESP_OK) { + if (err == ESP_ERR_NVS_NOT_FOUND) { + break; + } + return err; + } + err = findPage->cmpItem(nsIndex, ItemType::BLOB_DATA, key, static_cast(data) + offset, item.varLength.dataSize, static_cast (chunkStart) + chunkNum); + if (err != ESP_OK) { + return err; + } + assert(static_cast (chunkStart) + chunkNum == item.chunkIndex); + offset += item.varLength.dataSize; + } + if (err == ESP_OK) { + assert(offset == dataSize); + } + return err; +} + esp_err_t Storage::readItem(uint8_t nsIndex, ItemType datatype, const char* key, void* data, size_t dataSize) { if (mState != StorageState::ACTIVE) { diff --git a/components/nvs_flash/src/nvs_storage.hpp b/components/nvs_flash/src/nvs_storage.hpp index 3a79e705cc..e5f7813260 100644 --- a/components/nvs_flash/src/nvs_storage.hpp +++ b/components/nvs_flash/src/nvs_storage.hpp @@ -109,6 +109,8 @@ public: esp_err_t readMultiPageBlob(uint8_t nsIndex, const char* key, void* data, size_t dataSize); + esp_err_t cmpMultiPageBlob(uint8_t nsIndex, const char* key, const void* data, size_t dataSize); + esp_err_t eraseMultiPageBlob(uint8_t nsIndex, const char* key, VerOffset chunkStart = VerOffset::VER_ANY); void debugDump(); diff --git a/components/nvs_flash/test_nvs_host/test_nvs.cpp b/components/nvs_flash/test_nvs_host/test_nvs.cpp index 90ff02e061..126b1bc286 100644 --- a/components/nvs_flash/test_nvs_host/test_nvs.cpp +++ b/components/nvs_flash/test_nvs_host/test_nvs.cpp @@ -376,8 +376,8 @@ TEST_CASE("storage doesn't add duplicates within one page", "[nvs]") emu.setBounds(4, 8); CHECK(storage.init(4, 4) == ESP_OK); int bar = 0; - CHECK(storage.writeItem(1, "bar", bar) == ESP_OK); - CHECK(storage.writeItem(1, "bar", bar) == ESP_OK); + CHECK(storage.writeItem(1, "bar", ++bar) == ESP_OK); + CHECK(storage.writeItem(1, "bar", ++bar) == ESP_OK); Page page; page.load(4); @@ -404,11 +404,11 @@ TEST_CASE("storage doesn't add duplicates within multiple pages", "[nvs]") emu.setBounds(4, 8); CHECK(storage.init(4, 4) == ESP_OK); int bar = 0; - CHECK(storage.writeItem(1, "bar", bar) == ESP_OK); + CHECK(storage.writeItem(1, "bar", ++bar) == ESP_OK); for (size_t i = 0; i < Page::ENTRY_COUNT; ++i) { - CHECK(storage.writeItem(1, "foo", static_cast(bar)) == ESP_OK); + CHECK(storage.writeItem(1, "foo", static_cast(++bar)) == ESP_OK); } - CHECK(storage.writeItem(1, "bar", bar) == ESP_OK); + CHECK(storage.writeItem(1, "bar", ++bar) == ESP_OK); Page page; page.load(4); @@ -750,6 +750,58 @@ TEST_CASE("wifi test", "[nvs]") } +TEST_CASE("writing the identical content does not write or erase", "[nvs]") +{ + SpiFlashEmulator emu(20); + + const uint32_t NVS_FLASH_SECTOR = 5; + const uint32_t NVS_FLASH_SECTOR_COUNT_MIN = 10; + emu.setBounds(NVS_FLASH_SECTOR, NVS_FLASH_SECTOR + NVS_FLASH_SECTOR_COUNT_MIN); + TEST_ESP_OK(nvs_flash_init_custom(NVS_DEFAULT_PART_NAME, NVS_FLASH_SECTOR, NVS_FLASH_SECTOR_COUNT_MIN)); + + nvs_handle misc_handle; + TEST_ESP_OK(nvs_open("test", NVS_READWRITE, &misc_handle)); + + // Test writing a u8 twice, then changing it + nvs_set_u8(misc_handle, "test_u8", 8); + emu.clearStats(); + nvs_set_u8(misc_handle, "test_u8", 8); + CHECK(emu.getWriteOps() == 0); + CHECK(emu.getEraseOps() == 0); + CHECK(emu.getReadOps() != 0); + emu.clearStats(); + nvs_set_u8(misc_handle, "test_u8", 9); + CHECK(emu.getWriteOps() != 0); + CHECK(emu.getReadOps() != 0); + + // Test writing a string twice, then changing it + static const char *test[2] = {"Hello world.", "Hello world!"}; + nvs_set_str(misc_handle, "test_str", test[0]); + emu.clearStats(); + nvs_set_str(misc_handle, "test_str", test[0]); + CHECK(emu.getWriteOps() == 0); + CHECK(emu.getEraseOps() == 0); + CHECK(emu.getReadOps() != 0); + emu.clearStats(); + nvs_set_str(misc_handle, "test_str", test[1]); + CHECK(emu.getWriteOps() != 0); + CHECK(emu.getReadOps() != 0); + + // Test writing a multi-page blob, then changing it + uint8_t blob[Page::CHUNK_MAX_SIZE * 3] = {0}; + memset(blob, 1, sizeof(blob)); + nvs_set_blob(misc_handle, "test_blob", blob, sizeof(blob)); + emu.clearStats(); + nvs_set_blob(misc_handle, "test_blob", blob, sizeof(blob)); + CHECK(emu.getWriteOps() == 0); + CHECK(emu.getEraseOps() == 0); + CHECK(emu.getReadOps() != 0); + blob[sizeof(blob) - 1]++; + emu.clearStats(); + nvs_set_blob(misc_handle, "test_blob", blob, sizeof(blob)); + CHECK(emu.getWriteOps() != 0); + CHECK(emu.getReadOps() != 0); +} TEST_CASE("can init storage from flash with random contents", "[nvs]") {