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

@@ -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<unsigned>(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<unsigned>(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--;

components/nvs_flash/test/spi_flash_emulation.h

@@ -141,6 +141,18 @@ public:
     {
         return reinterpret_cast<const uint8_t*>(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_t>(size / SPI_FLASH_SEC_SIZE));
+    }
 
     void clearStats()
     {

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<uint8_t>(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<const uint32_t*>(&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;