From d9cdc7de5811a450919518b5ea8d6a8edd0a960f Mon Sep 17 00:00:00 2001 From: Ivan Grokhotkov Date: Mon, 31 Oct 2016 19:48:28 +0800 Subject: [PATCH] nvs_flash: don't allow more operation to be done on page in PageState::INVALID Currently a restart is required to recover a page from invalid state. The long-term solution is to detect such a condition and recover automatically (without a restart). This will be implemented in a separate change set. --- components/nvs_flash/src/nvs_page.cpp | 26 +++++--- .../nvs_flash/test/spi_flash_emulation.h | 12 ++++ components/nvs_flash/test/test_nvs.cpp | 62 +++++++++++++++++++ 3 files changed, 93 insertions(+), 7 deletions(-) diff --git a/components/nvs_flash/src/nvs_page.cpp b/components/nvs_flash/src/nvs_page.cpp index fae1f6f1b7..a10b88c976 100644 --- a/components/nvs_flash/src/nvs_page.cpp +++ b/components/nvs_flash/src/nvs_page.cpp @@ -3,7 +3,7 @@ // 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 - +// // http://www.apache.org/licenses/LICENSE-2.0 // // Unless required by applicable law or agreed to in writing, software @@ -131,8 +131,12 @@ esp_err_t Page::writeEntryData(const uint8_t* data, size_t size) esp_err_t Page::writeItem(uint8_t nsIndex, ItemType datatype, const char* key, const void* data, size_t dataSize) { Item item; - esp_err_t err; + + if (mState == PageState::INVALID) { + return ESP_ERR_NVS_INVALID_STATE; + } + if (mState == PageState::UNINITIALIZED) { err = initialize(); if (err != ESP_OK) { @@ -166,7 +170,6 @@ esp_err_t Page::writeItem(uint8_t nsIndex, ItemType datatype, const char* key, c } // write first item - size_t span = (totalSize + ENTRY_SIZE - 1) / ENTRY_SIZE; item = Item(nsIndex, datatype, span, key); mHashList.insert(item, mNextFreeEntry); @@ -215,6 +218,11 @@ esp_err_t Page::readItem(uint8_t nsIndex, ItemType datatype, const char* key, vo { 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); if (rc != ESP_OK) { return rc; @@ -478,6 +486,8 @@ esp_err_t Page::mLoadEntryTable() mState = PageState::INVALID; return err; } + + mHashList.insert(item, i); if (item.crc32 != item.calculateCrc32()) { err = eraseEntryAndSpan(i); @@ -488,8 +498,6 @@ esp_err_t Page::mLoadEntryTable() continue; } - mHashList.insert(item, i); - if (item.datatype != ItemType::BLOB && item.datatype != ItemType::SZ) { continue; } @@ -785,8 +793,12 @@ void Page::debugDump() const Item item; readEntry(i, item); if (skip == 0) { - printf("W ns=%2u type=%2u span=%3u key=\"%s\"\n", item.nsIndex, static_cast(item.datatype), item.span, item.key); - skip = item.span - 1; + printf("W ns=%2u type=%2u span=%3u key=\"%s\" len=%d\n", item.nsIndex, static_cast(item.datatype), item.span, item.key, (item.span != 1)?((int)item.varLength.dataSize):-1); + if (item.span > 0 && item.span <= ENTRY_COUNT - i) { + skip = item.span - 1; + } else { + skip = 0; + } } else { printf("D\n"); skip--; diff --git a/components/nvs_flash/test/spi_flash_emulation.h b/components/nvs_flash/test/spi_flash_emulation.h index 4e544a39e2..14e56bab6e 100644 --- a/components/nvs_flash/test/spi_flash_emulation.h +++ b/components/nvs_flash/test/spi_flash_emulation.h @@ -141,6 +141,18 @@ public: { return reinterpret_cast(mData.data()); } + + void load(const char* filename) + { + FILE* f = fopen(filename, "rb"); + fseek(f, 0, SEEK_END); + off_t size = ftell(f); + assert(size % SPI_FLASH_SEC_SIZE == 0); + mData.resize(size); + fseek(f, 0, SEEK_SET); + auto s = fread(mData.data(), SPI_FLASH_SEC_SIZE, size / SPI_FLASH_SEC_SIZE, f); + assert(s == static_cast(size / SPI_FLASH_SEC_SIZE)); + } void clearStats() { diff --git a/components/nvs_flash/test/test_nvs.cpp b/components/nvs_flash/test/test_nvs.cpp index 223e5dea9a..b70801f848 100644 --- a/components/nvs_flash/test/test_nvs.cpp +++ b/components/nvs_flash/test/test_nvs.cpp @@ -953,6 +953,68 @@ TEST_CASE("test for memory leaks in open/set", "[leaks]") } } +TEST_CASE("duplicate items are removed", "[nvs][dupes]") +{ + SpiFlashEmulator emu(3); + { + // create one item + nvs::Page p; + p.load(0); + p.writeItem(1, "opmode", 3); + } + { + // add another one without deleting the first one + nvs::Item item(1, ItemType::U8, 1, "opmode"); + item.data[0] = 2; + item.crc32 = item.calculateCrc32(); + emu.write(3 * 32, reinterpret_cast(&item), sizeof(item)); + uint32_t mask = 0xFFFFFFFA; + emu.write(32, &mask, 4); + } + { + // load page and check that second item persists + nvs::Page p; + p.load(0); + uint8_t val; + p.readItem(1, "opmode", val); + CHECK(val == 2); + CHECK(p.getErasedEntryCount() == 1); + CHECK(p.getUsedEntryCount() == 1); + } +} + +TEST_CASE("recovery after failure to write data", "[nvs]") +{ + SpiFlashEmulator emu(3); + // TODO: remove explicit alignment + const char str[] __attribute__((aligned(4))) = "value 0123456789abcdef012345678value 0123456789abcdef012345678"; + + // make flash write fail exactly in Page::writeEntryData + emu.failAfter(17); + { + Storage storage; + TEST_ESP_OK(storage.init(0, 3)); + + TEST_ESP_ERR(storage.writeItem(1, ItemType::SZ, "key", str, strlen(str)), ESP_ERR_FLASH_OP_FAIL); + + // check that repeated operations cause an error + TEST_ESP_ERR(storage.writeItem(1, ItemType::SZ, "key", str, strlen(str)), ESP_ERR_NVS_INVALID_STATE); + + uint8_t val; + TEST_ESP_ERR(storage.readItem(1, ItemType::U8, "key", &val, sizeof(val)), ESP_ERR_NVS_NOT_FOUND); + } + { + // load page and check that data was erased + Page p; + p.load(0); + CHECK(p.getErasedEntryCount() == 3); + CHECK(p.getUsedEntryCount() == 0); + + // try to write again + TEST_ESP_OK(p.writeItem(1, ItemType::SZ, "key", str, strlen(str))); + } +} + TEST_CASE("dump all performance data", "[nvs]") { std::cout << "====================" << std::endl << "Dumping benchmarks" << std::endl;