feat(nvs_flash): Update nvs_flash from esp-idf

MR !14548.

components/nvs_flash/host_test/fixtures/test_fixtures.hpp

@@ -52,7 +52,7 @@ public:
 
     esp_err_t read_raw(size_t src_offset, void* dst, size_t size) override
     {
-        return esp_partition_read(&partition, src_offset, dst, size);
+        return esp_partition_read_raw(&partition, src_offset, dst, size);
     }
 
     esp_err_t read(size_t src_offset, void* dst, size_t size) override
@@ -62,7 +62,7 @@ public:
 
     esp_err_t write_raw(size_t dst_offset, const void* src, size_t size) override
     {
-        return esp_partition_write(&partition, dst_offset, src, size);
+        return esp_partition_write_raw(&partition, dst_offset, src, size);
     }
 
     esp_err_t write(size_t dst_offset, const void* src, size_t size) override
@@ -122,6 +122,9 @@ struct PartitionMockFixture {
             const char *partition_name = NVS_DEFAULT_PART_NAME)
         : part_mock(start_sector * SPI_FLASH_SEC_SIZE, sector_size * SPI_FLASH_SEC_SIZE) {
         std::fill_n(raw_header, sizeof(raw_header)/sizeof(raw_header[0]), UINT8_MAX);
+
+        // This resets the mocks and prevents meeting accidental expectations from previous tests.
+        Mockesp_partition_Init();
     }
 
     ~PartitionMockFixture() { }
@@ -151,7 +154,7 @@ struct NVSPageFixture : public PartitionMockFixture {
     nvs::Page page;
 };
 
-struct NVSValidPageFixture : public PartitionMockFixture {
+struct NVSValidPageFlashFixture : public PartitionMockFixture {
     const static uint8_t NS_INDEX = 1;
 
     // valid header
@@ -164,7 +167,7 @@ struct NVSValidPageFixture : public PartitionMockFixture {
 
     uint8_t value_entry [32];
 
-    NVSValidPageFixture(uint32_t start_sector = 0,
+    NVSValidPageFlashFixture(uint32_t start_sector = 0,
             uint32_t sector_size = 1,
             const char *partition_name = NVS_DEFAULT_PART_NAME)
         : PartitionMockFixture(start_sector, sector_size, partition_name),
@@ -173,8 +176,7 @@ struct NVSValidPageFixture : public PartitionMockFixture {
         ns_entry {0x00, 0x01, 0x01, 0xff, 0x68, 0xc5, 0x3f, 0x0b, 't', 'e', 's', 't', '_', 'n', 's', '\0',
                 '\0', '\0', '\0', '\0', '\0', '\0', '\0', '\0', 1, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff},
         value_entry {0x01, 0x01, 0x01, 0xff, 0x3d, 0xf3, 0x99, 0xe5, 't', 'e', 's', 't', '_', 'v', 'a', 'l',
-                'u', 'e', '\0', '\0', '\0', '\0', '\0', '\0', 47, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff},
-        page()
+                'u', 'e', '\0', '\0', '\0', '\0', '\0', '\0', 47, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff}
     {
         std::fill_n(raw_entry_table, sizeof(raw_entry_table)/sizeof(raw_entry_table[0]), 0);
         raw_entry_table[0] = 0xfa;
@@ -202,7 +204,15 @@ struct NVSValidPageFixture : public PartitionMockFixture {
         // read normal entry second time during duplicated entry check
         esp_partition_read_ExpectAnyArgsAndReturn(ESP_OK);
         esp_partition_read_ReturnArrayThruPtr_dst(value_entry, 32);
+    }
+};
 
+struct NVSValidPageFixture : public NVSValidPageFlashFixture {
+    NVSValidPageFixture(uint32_t start_sector = 0,
+            uint32_t sector_size = 1,
+            const char *partition_name = NVS_DEFAULT_PART_NAME)
+        : NVSValidPageFlashFixture(start_sector, sector_size, partition_name), page()
+    {
         if (page.load(&part_mock, start_sector) != ESP_OK) throw FixtureException("couldn't setup page");
     }
 
@@ -392,9 +402,6 @@ struct NVSFullPageFixture : public PartitionMockFixture {
                 'u', 'e', '\0', '\0', '\0', '\0', '\0', '\0', 47, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff},
         page()
     {
-        std::fill_n(raw_entry_table, sizeof(raw_entry_table)/sizeof(raw_entry_table[0]), 0);
-        raw_entry_table[0] = 0xfa;
-
         // entry_table with all elements deleted except the namespace entry written and the last entry free
         std::fill_n(raw_entry_table, sizeof(raw_entry_table)/sizeof(raw_entry_table[0]), 0);
         raw_entry_table[0] = 0x0a;

components/nvs_flash/host_test/nvs_page_test/main/nvs_page_test.cpp

@@ -1,11 +1,16 @@
-/* Hello World Example
+// Copyright 2015-2016 Espressif Systems (Shanghai) PTE LTD
+//
+// 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
 
-   This example code is in the Public Domain (or CC0 licensed, at your option.)
-
-   Unless required by applicable law or agreed to in writing, this
-   software is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR
-   CONDITIONS OF ANY KIND, either express or implied.
-*/
+//     http://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.
 #include <stdio.h>
 #include "unity.h"
 #include "test_fixtures.hpp"
@@ -25,6 +30,8 @@ void test_Page_load_reading_header_fails()
 
     TEST_ASSERT_EQUAL(Page::PageState::INVALID, page.state());
     TEST_ASSERT_EQUAL(ESP_ERR_INVALID_ARG, page.load(&mock, 0));
+
+    Mockesp_partition_Verify();
 }
 
 void test_Page_load_reading_data_fails()
@@ -39,6 +46,8 @@ void test_Page_load_reading_data_fails()
 
     TEST_ASSERT_EQUAL(Page::PageState::INVALID, page.state());
     TEST_ASSERT_EQUAL(ESP_FAIL, page.load(&mock, 0));
+
+    Mockesp_partition_Verify();
 }
 
 void test_Page_load__uninitialized_page_has_0xfe()
@@ -57,6 +66,8 @@ void test_Page_load__uninitialized_page_has_0xfe()
     TEST_ASSERT_EQUAL(ESP_OK, page.load(&fix.part_mock, 0));
 
     TEST_ASSERT_EQUAL(Page::PageState::CORRUPT, page.state());
+
+    Mockesp_partition_Verify();
 }
 
 void test_Page_load__initialized_corrupt_header()
@@ -74,6 +85,60 @@ void test_Page_load__initialized_corrupt_header()
     TEST_ASSERT_EQUAL(ESP_OK, page.load(&fix.part_mock, 0));
 
     TEST_ASSERT_EQUAL(Page::PageState::CORRUPT, page.state());
+
+    Mockesp_partition_Verify();
+}
+
+void test_Page_load__corrupt_entry_table()
+{
+    PartitionMockFixture fix;
+
+    // valid header
+    uint8_t raw_header_valid [32] = {0xfe, 0xff, 0xff, 0xff, 0x00, 0x00, 0x00, 0x00, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff,
+                0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xc2, 0x16, 0xdd, 0xdc};
+
+    // entry table with one entry
+    uint8_t raw_entry_table [32];
+
+    uint8_t ns_entry [32] = {0x00, 0x01, 0x01, 0xff, 0x68, 0xc5, 0x3f, 0x0b, 't', 'e', 's', 't', '_', 'n', 's', '\0',
+                '\0', '\0', '\0', '\0', '\0', '\0', '\0', '\0', 1, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff};
+
+    uint8_t raw_header[4] = {0xff, 0xff, 0xff, 0xff};
+    std::fill_n(raw_entry_table, sizeof(raw_entry_table)/sizeof(raw_entry_table[0]), 0);
+    raw_entry_table[0] = 0xfa;
+
+    // read page header
+    esp_partition_read_raw_ExpectAnyArgsAndReturn(ESP_OK);
+    esp_partition_read_raw_ReturnArrayThruPtr_dst(raw_header_valid, 32);
+
+    // read entry table
+    esp_partition_read_raw_ExpectAnyArgsAndReturn(ESP_OK);
+    esp_partition_read_raw_ReturnArrayThruPtr_dst(raw_entry_table, 32);
+
+    // read next free entry's header
+    esp_partition_read_raw_ExpectAnyArgsAndReturn(ESP_OK);
+    esp_partition_read_raw_ReturnArrayThruPtr_dst(raw_header, 4);
+
+    // read namespace entry
+    esp_partition_read_ExpectAnyArgsAndReturn(ESP_OK);
+    esp_partition_read_ReturnArrayThruPtr_dst(ns_entry, 32);
+
+    // we expect a raw word write from the partition in order to change the entry bits to erased (0)
+    esp_partition_write_raw_ExpectAndReturn(&fix.part_mock.partition, 32, nullptr, 4, ESP_OK);
+    esp_partition_write_raw_IgnoreArg_src();
+
+    // corrupt entry table as well as crc of corresponding item
+    raw_entry_table[0] = 0xf6;
+
+    Page page;
+
+    // Page::load() should return ESP_OK, but state has to be corrupt
+    TEST_ASSERT_EQUAL(ESP_OK, page.load(&fix.part_mock, 0));
+
+    TEST_ASSERT_EQUAL(Page::PageState::ACTIVE, page.state());
+    TEST_ASSERT_EQUAL(1, page.getUsedEntryCount());
+
+    Mockesp_partition_Verify();
 }
 
 void test_Page_load_success()
@@ -881,6 +946,7 @@ int main(int argc, char **argv)
     RUN_TEST(test_Page_load_reading_data_fails);
     RUN_TEST(test_Page_load__uninitialized_page_has_0xfe);
     RUN_TEST(test_Page_load__initialized_corrupt_header);
+    RUN_TEST(test_Page_load__corrupt_entry_table);
     RUN_TEST(test_Page_load_success);
     RUN_TEST(test_Page_load_full_page);
     RUN_TEST(test_Page_load__seq_number_0);
@@ -929,6 +995,6 @@ int main(int argc, char **argv)
     RUN_TEST(test_Page_calcEntries__active_wo_blob);
     RUN_TEST(test_Page_calcEntries__active_with_blob);
     RUN_TEST(test_Page_calcEntries__invalid);
-    UNITY_END();
-    return 0;
+    int failures = UNITY_END();
+    return failures;
 }

components/nvs_flash/src/nvs_item_hash_list.cpp

@@ -65,7 +65,7 @@ esp_err_t HashList::insert(const Item& item, size_t index)
     return ESP_OK;
 }
 
-void HashList::erase(size_t index, bool itemShouldExist)
+bool HashList::erase(size_t index)
 {
     for (auto it = mBlockList.begin(); it != mBlockList.end();) {
         bool haveEntries = false;
@@ -81,7 +81,7 @@ void HashList::erase(size_t index, bool itemShouldExist)
             }
             if (haveEntries && foundIndex) {
                 /* item was found, and HashListBlock still has some items */
-                return;
+                return true;
             }
         }
         /* no items left in HashListBlock, can remove */
@@ -95,12 +95,12 @@ void HashList::erase(size_t index, bool itemShouldExist)
         }
         if (foundIndex) {
             /* item was found and empty HashListBlock was removed */
-            return;
+            return true;
         }
     }
-    if (itemShouldExist) {
-        assert(false && "item should have been present in cache");
-    }
+
+    // item hasn't been present in cache");
+    return false;
 }
 
 size_t HashList::find(size_t start, const Item& item)

components/nvs_flash/src/nvs_item_hash_list.hpp

@@ -29,7 +29,7 @@ public:
     ~HashList();
 
     esp_err_t insert(const Item& item, size_t index);
-    void erase(const size_t index, bool itemShouldExist=true);
+    bool erase(const size_t index);
     size_t find(size_t start, const Item& item);
     void clear();
 

components/nvs_flash/src/nvs_page.cpp

@@ -393,8 +393,9 @@ esp_err_t Page::findItem(uint8_t nsIndex, ItemType datatype, const char* key, ui
 
 esp_err_t Page::eraseEntryAndSpan(size_t index)
 {
+    uint32_t seq_num;
+    getSeqNumber(seq_num);
     auto state = mEntryTable.get(index);
-    assert(state == EntryState::WRITTEN || state == EntryState::EMPTY);
 
     size_t span = 1;
     if (state == EntryState::WRITTEN) {
@@ -404,7 +405,7 @@ esp_err_t Page::eraseEntryAndSpan(size_t index)
             return rc;
         }
         if (item.calculateCrc32() != item.crc32) {
-            mHashList.erase(index, false);
+            mHashList.erase(index);
             rc = alterEntryState(index, EntryState::ERASED);
             --mUsedEntryCount;
             ++mErasedEntryCount;
@@ -601,6 +602,16 @@ esp_err_t Page::mLoadEntryTable()
                 continue;
             }
 
+            if (mEntryTable.get(i) == static_cast<EntryState>(0x1)) {
+                lastItemIndex = INVALID_ENTRY;
+                auto err = eraseEntryAndSpan(i);
+                if (err != ESP_OK) {
+                    mState = PageState::INVALID;
+                    return err;
+                }
+                continue;
+            }
+
             lastItemIndex = i;
 
             auto err = readEntry(i, item);

components/nvs_flash/test_nvs_host/test_nvs.cpp

@@ -313,7 +313,8 @@ TEST_CASE("HashList is cleaned up as soon as items are erased", "[nvs]")
     INFO("Added " << count << " items, " << hashlist.getBlockCount() << " blocks");
     // Remove them in reverse order
     for (size_t i = count; i > 0; --i) {
-        hashlist.erase(i - 1, true);
+        // Make sure that the element existed before it's erased
+        CHECK(hashlist.erase(i - 1) == true);
     }
     CHECK(hashlist.getBlockCount() == 0);
     // Add again
@@ -326,7 +327,7 @@ TEST_CASE("HashList is cleaned up as soon as items are erased", "[nvs]")
     INFO("Added " << count << " items, " << hashlist.getBlockCount() << " blocks");
     // Remove them in the same order
     for (size_t i = 0; i < count; ++i) {
-        hashlist.erase(i, true);
+        CHECK(hashlist.erase(i) == true);
     }
     CHECK(hashlist.getBlockCount() == 0);
 }