Merge branch 'bugfix/nvs_erase_more_graceful' into 'master'

NVS: more graceful behavior for erasing partitions

See merge request espressif/esp-idf!7885

components/nvs_flash/include/nvs_flash.h

@@ -87,32 +87,37 @@ esp_err_t nvs_flash_deinit_partition(const char* partition_label);
 /**
  * @brief Erase the default NVS partition
  *
- * Erases all contents of the default NVS partition (one with label "nvs"), which must be uninitialized.
+ * Erases all contents of the default NVS partition (one with label "nvs").
+ *
+ * @note If the partition is initialized, this function first de-initializes it. Afterwards, the partition has to
+ *       be initialized again to be used.
  *
  * @return
  *      - ESP_OK on success
  *      - ESP_ERR_NOT_FOUND if there is no NVS partition labeled "nvs" in the
  *        partition table
- *      - ESP_ERR_NVS_INVALID_STATE if the default partition is initialized already
+ *      - different error in case de-initialization fails (shouldn't happen)
  */
 esp_err_t nvs_flash_erase(void);
 
 /**
  * @brief Erase specified NVS partition
  *
- * Erase all content of a specified uninitialized NVS partition
+ * Erase all content of a specified NVS partition
  *
- * @param[in]  part_name    Name (label) of an uninitialized partition which should be erased
+ * @note If the partition is initialized, this function first de-initializes it. Afterwards, the partition has to
+ *       be initialized again to be used.
+ *
+ * @param[in]  part_name    Name (label) of the partition which should be erased
  *
  * @return
  *      - ESP_OK on success
  *      - ESP_ERR_NOT_FOUND if there is no NVS partition with the specified name
  *        in the partition table
- *      - ESP_ERR_NVS_INVALID_STATE if the partition with part_name is initialized already
+ *      - different error in case de-initialization fails (shouldn't happen)
  */
 esp_err_t nvs_flash_erase_partition(const char *part_name);
 
-
 /**
  * @brief Initialize the default NVS partition.
  *

components/nvs_flash/src/nvs_api.cpp

@@ -118,6 +118,14 @@ extern "C" esp_err_t nvs_flash_secure_init_custom(const char *partName, uint32_t
 }
 #endif
 
+static esp_err_t close_handles_and_deinit(const char* part_name)
+{
+    // Delete all corresponding open handles
+    s_nvs_handles.clearAndFreeNodes();
+
+    // Deinit partition
+    return NVSPartitionManager::get_instance()->deinit_partition(part_name);
+}
 
 #ifdef ESP_PLATFORM
 extern "C" esp_err_t nvs_flash_init_partition(const char *part_name)
@@ -163,8 +171,17 @@ extern "C" esp_err_t nvs_flash_secure_init(nvs_sec_cfg_t* cfg)
 
 extern "C" esp_err_t nvs_flash_erase_partition(const char *part_name)
 {
+    Lock::init();
+    Lock lock;
+
+    // if the partition is initialized, uninitialize it first
     if (NVSPartitionManager::get_instance()->lookup_storage_from_name(part_name)) {
-        return ESP_ERR_NVS_INVALID_STATE;
+        esp_err_t err = close_handles_and_deinit(part_name);
+
+        // only hypothetical/future case, deinit_partition() only fails if partition is uninitialized
+        if (err != ESP_OK) {
+            return err;
+        }
     }
 
     const esp_partition_t* partition = esp_partition_find_first(
@@ -187,11 +204,7 @@ extern "C" esp_err_t nvs_flash_deinit_partition(const char* partition_name)
     Lock::init();
     Lock lock;
 
-    // Delete all corresponding open handles // TODO: why all handles, not just the ones with partition_name?
+    return close_handles_and_deinit(partition_name);
-    s_nvs_handles.clearAndFreeNodes();
-
-    // Deinit partition
-    return NVSPartitionManager::get_instance()->deinit_partition(partition_name);
 }
 
 extern "C" esp_err_t nvs_flash_deinit(void)

components/nvs_flash/test/test_nvs.c

@@ -18,21 +18,25 @@
 
 static const char* TAG = "test_nvs";
 
-TEST_CASE("flash erase fails on initialized partition", "[nvs]")
+TEST_CASE("flash erase deinitializes initialized partition", "[nvs]")
 {
+    nvs_handle_t handle;
     esp_err_t err = nvs_flash_init();
     if (err == ESP_ERR_NVS_NO_FREE_PAGES || err == ESP_ERR_NVS_NEW_VERSION_FOUND) {
-        ESP_LOGW(TAG, "nvs_flash_init failed (0x%x), erasing partition and retrying", err);
+        nvs_flash_erase();
-        const esp_partition_t* nvs_partition = esp_partition_find_first(
-                ESP_PARTITION_TYPE_DATA, ESP_PARTITION_SUBTYPE_DATA_NVS, NULL);
-        assert(nvs_partition && "partition table must have an NVS partition");
-        ESP_ERROR_CHECK( esp_partition_erase_range(nvs_partition, 0, nvs_partition->size) );
         err = nvs_flash_init();
     }
     ESP_ERROR_CHECK( err );
 
-    TEST_ASSERT_EQUAL(ESP_ERR_NVS_INVALID_STATE, nvs_flash_erase());
+    TEST_ESP_OK(nvs_flash_init());
+    TEST_ESP_OK(nvs_open("uninit_ns", NVS_READWRITE, &handle));
+    nvs_close(handle);
+    TEST_ESP_OK(nvs_flash_erase());
 
+    // exptected: no partition is initialized since nvs_flash_erase() deinitialized the partition again
+    TEST_ESP_ERR(ESP_ERR_NVS_NOT_INITIALIZED, nvs_open("uninit_ns", NVS_READWRITE, &handle));
+
+    // just to be sure it's deinitialized in case of error and not affecting other tests
     nvs_flash_deinit();
 }
 
@@ -43,10 +47,7 @@ TEST_CASE("nvs deinit with open handle", "[nvs]")
     esp_err_t err = nvs_flash_init();
     if (err == ESP_ERR_NVS_NO_FREE_PAGES || err == ESP_ERR_NVS_NEW_VERSION_FOUND) {
         ESP_LOGW(TAG, "nvs_flash_init failed (0x%x), erasing partition and retrying", err);
-        const esp_partition_t* nvs_partition = esp_partition_find_first(
+        ESP_ERROR_CHECK(nvs_flash_erase());
-                ESP_PARTITION_TYPE_DATA, ESP_PARTITION_SUBTYPE_DATA_NVS, NULL);
-        assert(nvs_partition && "partition table must have an NVS partition");
-        ESP_ERROR_CHECK( esp_partition_erase_range(nvs_partition, 0, nvs_partition->size) );
         err = nvs_flash_init();
     }
     ESP_ERROR_CHECK( err );
@@ -61,10 +62,7 @@ TEST_CASE("various nvs tests", "[nvs]")
     esp_err_t err = nvs_flash_init();
     if (err == ESP_ERR_NVS_NO_FREE_PAGES || err == ESP_ERR_NVS_NEW_VERSION_FOUND) {
         ESP_LOGW(TAG, "nvs_flash_init failed (0x%x), erasing partition and retrying", err);
-        const esp_partition_t* nvs_partition = esp_partition_find_first(
+        ESP_ERROR_CHECK(nvs_flash_erase());
-                ESP_PARTITION_TYPE_DATA, ESP_PARTITION_SUBTYPE_DATA_NVS, NULL);
-        assert(nvs_partition && "partition table must have an NVS partition");
-        ESP_ERROR_CHECK( esp_partition_erase_range(nvs_partition, 0, nvs_partition->size) );
         err = nvs_flash_init();
     }
     ESP_ERROR_CHECK( err );
@@ -131,7 +129,7 @@ TEST_CASE("calculate used and free space", "[nvs]")
         const esp_partition_t* nvs_partition = esp_partition_find_first(
                 ESP_PARTITION_TYPE_DATA, ESP_PARTITION_SUBTYPE_DATA_NVS, NULL);
         assert(nvs_partition && "partition table must have an NVS partition");
-        ESP_ERROR_CHECK( esp_partition_erase_range(nvs_partition, 0, nvs_partition->size) );
+        ESP_ERROR_CHECK(nvs_flash_erase());
         err = nvs_flash_init();
     }
     ESP_ERROR_CHECK( err );