From: Ivan Grokhotkov <ivan@espressif.com>
Date: Sun, 28 Jan 2018 15:37:31 +0000 (+0800)
Subject: spiffs: fix esp_spiffs_format not working if partition is not mounted
X-Git-Tag: v3.1-beta1~429^2
X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=1103e1babaafff78f48c3aa73221d26daf4436a2;p=esp-idf

spiffs: fix esp_spiffs_format not working if partition is not mounted

Fixes https://github.com/espressif/esp-idf/issues/1547.
---

diff --git a/components/spiffs/esp_spiffs.c b/components/spiffs/esp_spiffs.c
index 83eba96ee4..c344e57764 100644
--- a/components/spiffs/esp_spiffs.c
+++ b/components/spiffs/esp_spiffs.c
@@ -357,8 +357,12 @@ esp_err_t esp_spiffs_info(const char* partition_label, size_t *total_bytes, size
 
 esp_err_t esp_spiffs_format(const char* partition_label)
 {
-    bool mount_on_success = false;
+    bool partition_was_mounted = false;
     int index;
+    /* If the partition is not mounted, need to create SPIFFS structures
+     * and mount the partition, unmount, format, delete SPIFFS structures.
+     * See SPIFFS wiki for the reason why.
+     */
     esp_err_t err = esp_spiffs_by_label(partition_label, &index);
     if (err != ESP_OK) {
         esp_vfs_spiffs_conf_t conf = {
@@ -371,23 +375,28 @@ esp_err_t esp_spiffs_format(const char* partition_label)
             return err;
         }
         err = esp_spiffs_by_label(partition_label, &index);
-        if (err != ESP_OK) {
-            return err;
-        }
-        esp_spiffs_free(&_efs[index]);
-        return ESP_OK;
+        assert(err == ESP_OK && "failed to get index of the partition just mounted");
     } else if (SPIFFS_mounted(_efs[index]->fs)) {
-        SPIFFS_unmount(_efs[index]->fs);
-        mount_on_success = true;
+        partition_was_mounted = true;
     }
+
+    SPIFFS_unmount(_efs[index]->fs);
+
     s32_t res = SPIFFS_format(_efs[index]->fs);
     if (res != SPIFFS_OK) {
         ESP_LOGE(TAG, "format failed, %i", SPIFFS_errno(_efs[index]->fs));
         SPIFFS_clearerr(_efs[index]->fs);
+        /* If the partition was previously mounted, but format failed, don't
+         * try to mount the partition back (it will probably fail). On the
+         * other hand, if it was not mounted, need to clean up.
+         */
+        if (!partition_was_mounted) {
+            esp_spiffs_free(&_efs[index]);
+        }
         return ESP_FAIL;
     }
 
-    if (mount_on_success) {
+    if (partition_was_mounted) {
         res = SPIFFS_mount(_efs[index]->fs, &_efs[index]->cfg, _efs[index]->work,
                             _efs[index]->fds, _efs[index]->fds_sz, _efs[index]->cache,
                             _efs[index]->cache_sz, spiffs_api_check);
@@ -396,6 +405,8 @@ esp_err_t esp_spiffs_format(const char* partition_label)
             SPIFFS_clearerr(_efs[index]->fs);
             return ESP_FAIL;
         }
+    } else {
+        esp_spiffs_free(&_efs[index]);
     }
     return ESP_OK;
 }
diff --git a/components/spiffs/test/test_spiffs.c b/components/spiffs/test/test_spiffs.c
index b428de4ceb..fdd3d241a6 100644
--- a/components/spiffs/test/test_spiffs.c
+++ b/components/spiffs/test/test_spiffs.c
@@ -407,7 +407,7 @@ static void test_teardown()
     TEST_ESP_OK(esp_vfs_spiffs_unregister(spiffs_test_partition_label));
 }
 
-TEST_CASE("can format partition", "[spiffs]")
+TEST_CASE("can initialize SPIFFS in erased partition", "[spiffs]")
 {
     const esp_partition_t* part = get_test_data_partition();
     TEST_ASSERT_NOT_NULL(part);
@@ -420,6 +420,44 @@ TEST_CASE("can format partition", "[spiffs]")
     test_teardown();
 }
 
+TEST_CASE("can format mounted partition", "[spiffs]")
+{
+    // Mount SPIFFS, create file, format, check that the file does not exist.
+    const esp_partition_t* part = get_test_data_partition();
+    TEST_ASSERT_NOT_NULL(part);
+    test_setup();
+    const char* filename = "/spiffs/hello.txt";
+    test_spiffs_create_file_with_text(filename, spiffs_test_hello_str);
+    esp_spiffs_format(part->label);
+    FILE* f = fopen(filename, "r");
+    TEST_ASSERT_NULL(f);
+    test_teardown();
+}
+
+TEST_CASE("can format unmounted partition", "[spiffs]")
+{
+    // Mount SPIFFS, create file, unmount. Format. Mount again, check that
+    // the file does not exist.
+    const esp_partition_t* part = get_test_data_partition();
+    TEST_ASSERT_NOT_NULL(part);
+    test_setup();
+    const char* filename = "/spiffs/hello.txt";
+    test_spiffs_create_file_with_text(filename, spiffs_test_hello_str);
+    test_teardown();
+    esp_spiffs_format(part->label);
+    // Don't use test_setup here, need to mount without formatting
+    esp_vfs_spiffs_conf_t conf = {
+        .base_path = "/spiffs",
+        .partition_label = spiffs_test_partition_label,
+        .max_files = 5,
+        .format_if_mount_failed = false
+    };
+    TEST_ESP_OK(esp_vfs_spiffs_register(&conf));
+    FILE* f = fopen(filename, "r");
+    TEST_ASSERT_NULL(f);
+    test_teardown();
+}
+
 TEST_CASE("can create and write file", "[spiffs]")
 {
     test_setup();