From 09d2c5daa651c1034251cce433cb54ed765d8f95 Mon Sep 17 00:00:00 2001
From: Jakob Hasse <jakob.hasse@espressif.com>
Date: Thu, 22 Oct 2020 10:27:42 +0800
Subject: [PATCH] nvs_flash: fixed deinit other partition's handles

* When deinitializing or erasing a partition,
  nvs used to close all handles instead of only
  the current partition's handles.
  This is fixed now
* Added a unit test for that case

Closes FCS-533
---
 components/nvs_flash/src/nvs_api.cpp          | 13 ++++++--
 .../nvs_flash/src/nvs_handle_simple.cpp       |  4 +++
 .../nvs_flash/src/nvs_handle_simple.hpp       |  2 ++
 .../nvs_flash/test_nvs_host/test_nvs.cpp      | 31 +++++++++++++++++++
 4 files changed, 48 insertions(+), 2 deletions(-)

diff --git a/components/nvs_flash/src/nvs_api.cpp b/components/nvs_flash/src/nvs_api.cpp
index 8ce0d07ba5..423e1fdceb 100644
--- a/components/nvs_flash/src/nvs_api.cpp
+++ b/components/nvs_flash/src/nvs_api.cpp
@@ -19,6 +19,7 @@
 #include "nvs_partition_manager.hpp"
 #include "esp_partition.h"
 #include "sdkconfig.h"
+#include <functional>
 #include "nvs_handle_simple.hpp"
 
 #ifdef ESP_PLATFORM
@@ -84,8 +85,16 @@ extern "C" void nvs_dump(const char *partName)
 
 static esp_err_t close_handles_and_deinit(const char* part_name)
 {
-    // Delete all corresponding open handles
-    s_nvs_handles.clearAndFreeNodes();
+    auto belongs_to_part = [=](NVSHandleEntry& e) -> bool {
+        return e.nvs_handle->get_partition_name() == part_name;
+    };
+
+    auto it = find_if(begin(s_nvs_handles), end(s_nvs_handles), belongs_to_part);
+
+    while (it != end(s_nvs_handles)) {
+        s_nvs_handles.erase(it);
+        it = find_if(begin(s_nvs_handles), end(s_nvs_handles), belongs_to_part);
+    }
 
     // Deinit partition
     return NVSPartitionManager::get_instance()->deinit_partition(part_name);
diff --git a/components/nvs_flash/src/nvs_handle_simple.cpp b/components/nvs_flash/src/nvs_handle_simple.cpp
index 744dd80fb4..348e197b7c 100644
--- a/components/nvs_flash/src/nvs_handle_simple.cpp
+++ b/components/nvs_flash/src/nvs_handle_simple.cpp
@@ -130,4 +130,8 @@ bool NVSHandleSimple::nextEntry(nvs_opaque_iterator_t* it) {
     return mStoragePtr->nextEntry(it);
 }
 
+const char *NVSHandleSimple::get_partition_name() const {
+    return mStoragePtr->getPartName();
+}
+
 }
diff --git a/components/nvs_flash/src/nvs_handle_simple.hpp b/components/nvs_flash/src/nvs_handle_simple.hpp
index fc3cebcd41..0a20aa4e8f 100644
--- a/components/nvs_flash/src/nvs_handle_simple.hpp
+++ b/components/nvs_flash/src/nvs_handle_simple.hpp
@@ -76,6 +76,8 @@ public:
 
     bool nextEntry(nvs_opaque_iterator_t *it);
 
+    const char *get_partition_name() const;
+
 private:
     /**
      * The underlying storage's object.
diff --git a/components/nvs_flash/test_nvs_host/test_nvs.cpp b/components/nvs_flash/test_nvs_host/test_nvs.cpp
index 4749076cb4..d8467dfe57 100644
--- a/components/nvs_flash/test_nvs_host/test_nvs.cpp
+++ b/components/nvs_flash/test_nvs_host/test_nvs.cpp
@@ -705,6 +705,37 @@ TEST_CASE("nvs api tests", "[nvs]")
     TEST_ESP_OK(nvs_flash_deinit_partition(NVS_DEFAULT_PART_NAME));
 }
 
+TEST_CASE("deinit partition doesn't affect other partition's open handles", "[nvs]")
+{
+    const char *OTHER_PARTITION_NAME = "other_part";
+    PartitionEmulationFixture f(0, 10);
+    PartitionEmulationFixture f_other(0, 10, OTHER_PARTITION_NAME);
+    const char* str = "value 0123456789abcdef0123456789abcdef";
+    const uint8_t blob[8] = {0x0, 0x1, 0x2, 0x3, 0x4, 0x5, 0x6, 0x7};
+
+    nvs_handle_t handle_1;
+    const uint32_t NVS_FLASH_SECTOR = 6;
+    const uint32_t NVS_FLASH_SECTOR_COUNT_MIN = 3;
+    f.emu.setBounds(NVS_FLASH_SECTOR, NVS_FLASH_SECTOR + NVS_FLASH_SECTOR_COUNT_MIN);
+    f_other.emu.setBounds(NVS_FLASH_SECTOR, NVS_FLASH_SECTOR + NVS_FLASH_SECTOR_COUNT_MIN);
+
+    TEST_ESP_OK(NVSPartitionManager::get_instance()->init_custom(&f.part,
+            NVS_FLASH_SECTOR,
+            NVS_FLASH_SECTOR_COUNT_MIN));
+    TEST_ESP_OK(NVSPartitionManager::get_instance()->init_custom(&f_other.part,
+            NVS_FLASH_SECTOR,
+            NVS_FLASH_SECTOR_COUNT_MIN));
+
+    TEST_ESP_OK(nvs_open_from_partition(OTHER_PARTITION_NAME, "ns", NVS_READWRITE, &handle_1));
+
+    // Deinitializing must not interfere with the open handle from the other partition.
+    TEST_ESP_OK(nvs_flash_deinit_partition(NVS_DEFAULT_PART_NAME));
+
+    TEST_ESP_OK(nvs_set_i32(handle_1, "foo", 0x3456789a));
+    nvs_close(handle_1);
+
+    TEST_ESP_OK(nvs_flash_deinit_partition(OTHER_PARTITION_NAME));
+}
 
 TEST_CASE("nvs iterators tests", "[nvs]")
 {