Merge branch 'bugfix/nvs_init_deinit' into 'master'

nvs: fix bug that NVS is not initialized after init failure, add deinit functions



See merge request !1282

components/nvs_flash/include/nvs_flash.h

@@ -24,7 +24,7 @@ extern "C" {
  * @brief Initialize the default NVS partition.
  *
  * This API initialises the default NVS partition. The default NVS partition
- * is the one that is labelled "nvs" in the partition table.
+ * is the one that is labeled "nvs" in the partition table.
  *
  * @return
  *      - ESP_OK if storage was successfully initialized.
@@ -38,7 +38,7 @@ esp_err_t nvs_flash_init(void);
 /**
  * @brief Initialize NVS flash storage for the specified partition.
  *
- * @param[in]  partition_name    Name (label) of the partition. Note that internally a reference to
+ * @param[in]  partition_label   Label of the partition. Note that internally a reference to
  *                               passed value is kept and it should be accessible for future operations
  *
  * @return
@@ -48,7 +48,30 @@ esp_err_t nvs_flash_init(void);
  *      - ESP_ERR_NOT_FOUND if specified partition is not found in the partition table
  *      - one of the error codes from the underlying flash storage driver
  */
-esp_err_t nvs_flash_init_partition(const char *partition_name);
+esp_err_t nvs_flash_init_partition(const char *partition_label);
+
+/**
+ * @brief Deinitialize NVS storage for the default NVS partition
+ *
+ * Default NVS partition is the partition with "nvs" label in the partition table.
+ *
+ * @return
+ *      - ESP_OK on success (storage was deinitialized)
+ *      - ESP_ERR_NVS_NOT_INITIALIZED if the storage was not initialized prior to this call
+ */
+esp_err_t nvs_flash_deinit(void);
+
+/**
+ * @brief Deinitialize NVS storage for the given NVS partition
+ *
+ * @param[in]  partition_label   Label of the partition
+ *
+ * @return
+ *      - ESP_OK on success
+ *      - ESP_ERR_NVS_NOT_INITIALIZED if the storage for given partition was not
+ *        initialized prior to this call
+ */
+esp_err_t nvs_flash_deinit_partition(const char* partition_label);
 
 /**
  * @brief Erase the default NVS partition

components/nvs_flash/src/nvs_api.cpp

@@ -88,15 +88,22 @@ extern "C" void nvs_dump(const char *partName)
 extern "C" esp_err_t nvs_flash_init_custom(const char *partName, uint32_t baseSector, uint32_t sectorCount)
 {
     ESP_LOGD(TAG, "nvs_flash_init_custom partition=%s start=%d count=%d", partName, baseSector, sectorCount);
-    nvs::Storage* mStorage;
-
-    mStorage = lookup_storage_from_name(partName);
-    if (mStorage == NULL) {
-        mStorage = new nvs::Storage((const char *)partName);
-        s_nvs_storage_list.push_back(mStorage);
+    nvs::Storage* new_storage = NULL;
+    nvs::Storage* storage = lookup_storage_from_name(partName);
+    if (storage == NULL) {
+        new_storage = new nvs::Storage((const char *)partName);
+        storage = new_storage;
     }
 
-    return mStorage->init(baseSector, sectorCount);
+    esp_err_t err = storage->init(baseSector, sectorCount);
+    if (new_storage != NULL) {
+        if (err == ESP_OK) {
+            s_nvs_storage_list.push_back(new_storage);
+        } else {
+            delete new_storage;
+        }
+    }
+    return err;
 }
 
 #ifdef ESP_PLATFORM
@@ -126,6 +133,42 @@ extern "C" esp_err_t nvs_flash_init(void)
     return nvs_flash_init_partition(NVS_DEFAULT_PART_NAME);
 }
 
+extern "C" esp_err_t nvs_flash_deinit_partition(const char* partition_name)
+{
+    Lock::init();
+    Lock lock;
+
+    nvs::Storage* storage = lookup_storage_from_name(partition_name);
+    if (!storage) {
+        return ESP_ERR_NVS_NOT_INITIALIZED;
+    }
+
+    /* Clean up handles related to the storage being deinitialized */
+    auto it = s_nvs_handles.begin();
+    auto next = it;
+    while(it != s_nvs_handles.end()) {
+        next++;
+        if (it->mStoragePtr == storage) {
+            ESP_LOGD(TAG, "Deleting handle %d (ns=%d) related to partition \"%s\" (missing call to nvs_close?)",
+                    it->mHandle, it->mNsIndex, partition_name);
+            s_nvs_handles.erase(it);
+            delete static_cast<HandleEntry*>(it);
+        }
+        it = next;
+    }
+
+    /* Finally delete the storage itself */
+    s_nvs_storage_list.erase(storage);
+    delete storage;
+
+    return ESP_OK;
+}
+
+extern "C" esp_err_t nvs_flash_deinit(void)
+{
+    return nvs_flash_deinit_partition(NVS_DEFAULT_PART_NAME);
+}
+
 extern "C" esp_err_t nvs_flash_erase_partition(const char *part_name)
 {
     const esp_partition_t* partition = esp_partition_find_first(

components/nvs_flash/test/test_nvs.c

@@ -59,5 +59,9 @@ TEST_CASE("various nvs tests", "[nvs]")
     TEST_ASSERT_EQUAL_INT32(0, strcmp(buf, str));
 
     nvs_close(handle_1);
+
+    // check that deinit does not leak memory if some handles are still open
+    nvs_flash_deinit();
+
     nvs_close(handle_2);
 }

components/nvs_flash/test_nvs_host/spi_flash_emulation.h

@@ -148,10 +148,20 @@ public:
         fseek(f, 0, SEEK_END);
         off_t size = ftell(f);
         assert(size % SPI_FLASH_SEC_SIZE == 0);
-        mData.resize(size);
+        mData.resize(size / sizeof(uint32_t));
         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));
+        fclose(f);
+    }
+    
+    void save(const char* filename)
+    {
+        FILE* f = fopen(filename, "wb");
+        auto n_sectors = mData.size() * sizeof(uint32_t) / SPI_FLASH_SEC_SIZE;
+        auto s = fwrite(mData.data(), SPI_FLASH_SEC_SIZE, n_sectors, f);
+        assert(s == n_sectors);
+        fclose(f);
     }
 
     void clearStats()

tools/unit-test-app/components/unity/unity_platform.c

@@ -10,6 +10,7 @@
 #include "esp_log.h"
 #include "soc/cpu.h"
 #include "esp_heap_caps.h"
+#include "test_utils.h"
 
 #define unity_printf ets_printf
 
@@ -37,6 +38,7 @@ const size_t CRITICAL_LEAK_THRESHOLD = 4096;
 void setUp(void)
 {
     printf("%s", ""); /* sneakily lazy-allocate the reent structure for this test task */
+    get_test_data_partition();  /* allocate persistent partition table structures */
 
     before_free_8bit = heap_caps_get_free_size(MALLOC_CAP_8BIT);
     before_free_32bit = heap_caps_get_free_size(MALLOC_CAP_32BIT);