fix(storage): FATFS WL function formatting wrong partition

Closes https://github.com/espressif/esp-idf/issues/12542

Co-authored-by: Tony Stuart <anthonyfstuart@gmail.com>

components/fatfs/host_test/main/test_fatfs.cpp

@@ -14,7 +14,7 @@
 
 #include "catch.hpp"
 
-TEST_CASE("create volume, open file, write and read back data", "[fatfs]")
+TEST_CASE("Create volume, open file, write and read back data", "[fatfs]")
 {
     FRESULT fr_result;
     BYTE pdrv;
@@ -90,6 +90,196 @@ TEST_CASE("create volume, open file, write and read back data", "[fatfs]")
     fr_result = f_mount(0, "", 0);
     REQUIRE(fr_result == FR_OK);
 
+    // Clear
     free(read);
     free(data);
+    ff_diskio_unregister(pdrv);
+    ff_diskio_clear_pdrv_wl(wl_handle);
+    esp_result = wl_unmount(wl_handle);
+    REQUIRE(esp_result == ESP_OK);
+}
+
+static void prepare_fatfs(const char* partition_label, const esp_partition_t** partition, wl_handle_t* wl_handle, BYTE* pdrv)
+{
+    FRESULT fr_result;
+    esp_err_t esp_result;
+
+    *partition = esp_partition_find_first(ESP_PARTITION_TYPE_DATA, ESP_PARTITION_SUBTYPE_DATA_FAT, partition_label);
+    REQUIRE(partition != NULL);
+    printf("partition address=0x%x\n", (*partition)->address);
+    printf("partition size=0x%x\n", (*partition)->size);
+
+    // Mount wear-levelled partition
+    esp_result = wl_mount(*partition, wl_handle);
+    REQUIRE(esp_result == ESP_OK);
+
+    // Get a physical drive
+    BYTE _pdrv;
+    esp_result = ff_diskio_get_drive(&_pdrv);
+    REQUIRE(esp_result == ESP_OK);
+    printf("using pdrv=%i\n", _pdrv);
+    char drv[3] = {(char)('0' + _pdrv), ':', 0};
+    *pdrv = _pdrv;
+
+    // Register physical drive as wear-levelled partition
+    esp_result = ff_diskio_register_wl_partition(_pdrv, *wl_handle);
+
+    // Create FAT volume on the entire disk
+    LBA_t part_list[] = {100, 0, 0, 0};
+    BYTE work_area[FF_MAX_SS];
+
+    fr_result = f_fdisk(_pdrv, part_list, work_area);
+    REQUIRE(fr_result == FR_OK);
+    const MKFS_PARM opt = {(BYTE)FM_ANY, 0, 0, 0, 0};
+    fr_result = f_mkfs(drv, &opt, work_area, sizeof(work_area)); // Use default volume
+    REQUIRE(fr_result == FR_OK);
+}
+
+/*
+ * This just tests formatting from FATFS library itself, not directly VFS FATFS (SPIFLASH) API
+ * like `esp_vfs_fat_spiflash_format_rw_wl` function, since `vfs` is not buildable on linux host
+ * at the time of writing this - therefore there also is a device test_apps test in
+ * `components/fatfs/test_apps/flash_wl/main/test_fatfs_flash_wl.c` which tests our VFS FATFS SPIFLASH API.
+ */
+TEST_CASE("Test mounting 2 volumes, writing data and formating the 2nd one, reading data", "[fatfs]")
+{
+    FRESULT fr_result;
+    esp_err_t esp_result;
+
+    const char* partition_label0 = "storage";
+    const esp_partition_t *partition0 = NULL;
+    BYTE pdrv0 = UINT8_MAX;
+    FATFS fs0;
+    wl_handle_t wl_handle0 = WL_INVALID_HANDLE;
+
+    const char* partition_label1 = "storage2";
+    const esp_partition_t *partition1 = NULL;
+    BYTE pdrv1 = UINT8_MAX;
+    FATFS fs1;
+    wl_handle_t wl_handle1 = WL_INVALID_HANDLE;
+
+    size_t allocation_unit_size = CONFIG_WL_SECTOR_SIZE;
+    size_t data_size = 10;
+
+
+    // Mount the volume 0
+    prepare_fatfs(partition_label0, &partition0, &wl_handle0, &pdrv0);
+    REQUIRE(partition0 != NULL);
+    REQUIRE(wl_handle0 != WL_INVALID_HANDLE);
+    REQUIRE(pdrv0 == 0);
+    char drv0[3] = {(char)('0' + pdrv0), ':', 0};
+    fr_result = f_mount(&fs0, drv0, 0);
+    REQUIRE(fr_result == FR_OK);
+
+    // Open file and write data
+    FIL file0;
+    UINT bw0;
+    fr_result = f_open(&file0, "0:/test0.txt", FA_OPEN_ALWAYS | FA_WRITE);
+    REQUIRE(fr_result == FR_OK);
+    // Write data
+    const char *data0 = "123456789";
+    char read0[10] = {0};
+    fr_result = f_write(&file0, data0, data_size, &bw0);
+    REQUIRE(fr_result == FR_OK);
+    REQUIRE(bw0 == data_size);
+    // Close file
+    fr_result = f_close(&file0);
+    REQUIRE(fr_result == FR_OK);
+
+    // Unmount volume 0
+    fr_result = f_mount(0, drv0, 0);
+    REQUIRE(fr_result == FR_OK);
+
+
+    // Mount the volume 1
+    prepare_fatfs(partition_label1, &partition1, &wl_handle1, &pdrv1);
+    REQUIRE(partition1 != NULL);
+    REQUIRE(wl_handle1 != WL_INVALID_HANDLE);
+    REQUIRE(pdrv1 == 1);
+    char drv1[3] = {(char)('0' + pdrv1), ':', 0};
+    fr_result = f_mount(&fs1, drv1, 0);
+    REQUIRE(fr_result == FR_OK);
+
+    // Open file and write data
+    FIL file1;
+    UINT bw1;
+    fr_result = f_open(&file1, "1:/test1.txt", FA_OPEN_ALWAYS | FA_WRITE);
+    REQUIRE(fr_result == FR_OK);
+    // Write data
+    const char* data1 = "987654321";
+    char read1[10] = {0};
+    fr_result = f_write(&file1, data1, data_size, &bw1);
+    REQUIRE(fr_result == FR_OK);
+    REQUIRE(bw1 == data_size);
+    // Close file
+    fr_result = f_close(&file1);
+    REQUIRE(fr_result == FR_OK);
+
+    // Unmount volume 1
+    fr_result = f_mount(0, drv1, 0);
+    REQUIRE(fr_result == FR_OK);
+
+    // Format the volume 1
+    const size_t workbuf_size = 4096;
+    void *workbuf = ff_memalloc(workbuf_size);
+    REQUIRE(workbuf != NULL);
+    const MKFS_PARM opt = {(BYTE)(FM_ANY | FM_SFD), 0, 0, 0, CONFIG_WL_SECTOR_SIZE};
+    fr_result = f_mkfs(drv1, &opt, workbuf, workbuf_size);
+    free(workbuf);
+    workbuf = NULL;
+    REQUIRE(fr_result == FR_OK);
+    printf("partition1 formatted\n");
+
+
+    // Remount the volume 1
+    fr_result = f_mount(&fs1, drv1, 1);
+    REQUIRE(fr_result == FR_OK);
+    // Open file and read data from file1
+    fr_result = f_open(&file1, "1:/test1.txt", FA_OPEN_ALWAYS | FA_READ);
+    REQUIRE(fr_result == FR_OK);
+    // Read written data from file1
+    fr_result = f_read(&file1, read1, data_size, &bw1);
+    REQUIRE(fr_result == FR_OK);
+    REQUIRE(bw1 != data_size);
+    // Comapre data
+    printf("data1=%s, read1=%s\n", data1, read1);
+    REQUIRE(strncmp(data1, read1, data_size-1) != 0); // 987654321 should be ersead due to formatting
+    // Close file from file1
+    fr_result = f_close(&file1);
+    REQUIRE(fr_result == FR_OK);
+
+
+    // Remount the volume 0
+    fr_result = f_mount(&fs0, drv0, 1);
+    REQUIRE(fr_result == FR_OK);
+    // Open file and read data from file0
+    fr_result = f_open(&file0, "0:/test0.txt", FA_OPEN_ALWAYS | FA_READ);
+    REQUIRE(fr_result == FR_OK);
+    // Read written data from file0
+    fr_result = f_read(&file0, read0, data_size, &bw0);
+    REQUIRE(fr_result == FR_OK);
+    REQUIRE(bw0 == data_size);
+    // Comapre data
+    printf("data0=%s, read0=%s\n", data0, read0);
+    REQUIRE(strncmp(data0, read0, data_size-1) == 0); // should match since the partition was not formatted
+    // Close file from file0
+    fr_result = f_close(&file0);
+    REQUIRE(fr_result == FR_OK);
+
+
+    // Unmount both volumes
+    fr_result = f_mount(0, drv0, 0);
+    REQUIRE(fr_result == FR_OK);
+    fr_result = f_mount(0, drv1, 0);
+    REQUIRE(fr_result == FR_OK);
+
+    // Clear
+    ff_diskio_unregister(pdrv0);
+    ff_diskio_unregister(pdrv1);
+    ff_diskio_clear_pdrv_wl(wl_handle0);
+    ff_diskio_clear_pdrv_wl(wl_handle1);
+    esp_result = wl_unmount(wl_handle0);
+    REQUIRE(esp_result == ESP_OK);
+    esp_result = wl_unmount(wl_handle1);
+    REQUIRE(esp_result == ESP_OK);
 }

components/fatfs/host_test/partition_table.csv

@@ -4,3 +4,4 @@ nvs,      data, nvs,     0x9000,  0x6000,
 phy_init, data, phy,     0xf000,  0x1000,
 factory,  app,  factory, 0x10000, 1M,
 storage,  data, fat,     ,        1M,
+storage2, data, fat,     ,        1M,

components/fatfs/host_test/sdkconfig.defaults

@@ -9,4 +9,4 @@ CONFIG_PARTITION_TABLE_CUSTOM_FILENAME="partition_table.csv"
 CONFIG_ESPTOOLPY_FLASHSIZE="8MB"
 CONFIG_MMU_PAGE_SIZE=0X10000
 CONFIG_ESP_PARTITION_ENABLE_STATS=y
-CONFIG_FATFS_VOLUME_COUNT=2
+CONFIG_FATFS_VOLUME_COUNT=3

components/fatfs/test_apps/flash_wl/main/test_fatfs_flash_wl.c

@@ -61,6 +61,30 @@ TEST_CASE("(WL) can format when the FAT is mounted already", "[fatfs][wear_level
     test_teardown();
 }
 
+TEST_CASE("(WL) can format specified FAT when more are mounted", "[fatfs][wear_levelling][timeout=180]")
+{
+    esp_vfs_fat_sdmmc_mount_config_t mount_config = {
+        .format_if_mount_failed = true,
+        .max_files = 5,
+    };
+    wl_handle_t s_test_wl_handle1;
+    wl_handle_t s_test_wl_handle2;
+    TEST_ESP_OK(esp_vfs_fat_spiflash_mount_rw_wl("/spiflash1", "storage", &mount_config, &s_test_wl_handle1));
+    TEST_ESP_OK(esp_vfs_fat_spiflash_mount_rw_wl("/spiflash2", "storage2", &mount_config, &s_test_wl_handle2));
+
+    test_fatfs_create_file_with_text("/spiflash1/hello.txt", fatfs_test_hello_str);
+    test_fatfs_create_file_with_text("/spiflash2/hello.txt", fatfs_test_hello_str);
+
+    TEST_ESP_OK(esp_vfs_fat_spiflash_format_rw_wl("/spiflash2", "storage2"));
+
+    FILE* f = fopen("/spiflash2/hello.txt", "r");
+    TEST_ASSERT_NULL(f); // File is erased on the formatted FAT
+    test_fatfs_pread_file("/spiflash1/hello.txt"); // File is still readable on the other FAT
+
+    TEST_ESP_OK(esp_vfs_fat_spiflash_unmount_rw_wl("/spiflash1", s_test_wl_handle1));
+    TEST_ESP_OK(esp_vfs_fat_spiflash_unmount_rw_wl("/spiflash2", s_test_wl_handle2));
+}
+
 TEST_CASE("(WL) can create and write file", "[fatfs][wear_levelling]")
 {
     test_setup();

components/fatfs/test_apps/flash_wl/partitions.csv

@@ -1,3 +1,4 @@
 # Name,   Type, SubType, Offset,  Size, Flags
-factory,  app,  factory, 0x10000, 1M,
+factory,  app,  factory, 0x10000, 768k,
 storage,  data, fat,     ,        528k,
+storage2, data, fat,     ,        528k,

components/fatfs/vfs/vfs_fat_sdmmc.c

@@ -1,11 +1,12 @@
 /*
- * SPDX-FileCopyrightText: 2015-2022 Espressif Systems (Shanghai) CO LTD
+ * SPDX-FileCopyrightText: 2015-2023 Espressif Systems (Shanghai) CO LTD
  *
  * SPDX-License-Identifier: Apache-2.0
  */
 
 #include <stdlib.h>
 #include <string.h>
+#include "esp_check.h"
 #include "esp_log.h"
 #include "esp_vfs.h"
 #include "esp_vfs_fat.h"
@@ -471,7 +472,8 @@ esp_err_t esp_vfs_fat_sdcard_format(const char *base_path, sdmmc_card_t *card)
 
     //unmount
     char drv[3] = {(char)('0' + pdrv), ':', 0};
-    f_mount(0, drv, 0);
+    FRESULT res = f_mount(0, drv, 0);
+    ESP_RETURN_ON_FALSE(res != FR_INVALID_DRIVE, ESP_FAIL, TAG, "f_mount unmount failed (%d) - the logical drive number is invalid", res);
 
     //format
     uint32_t id = FF_VOLUMES;
@@ -482,7 +484,7 @@ esp_err_t esp_vfs_fat_sdcard_format(const char *base_path, sdmmc_card_t *card)
                 s_ctx[id]->mount_config.allocation_unit_size);
     ESP_LOGI(TAG, "Formatting card, allocation unit size=%d", alloc_unit_size);
     const MKFS_PARM opt = {(BYTE)FM_ANY, 0, 0, 0, alloc_unit_size};
-    FRESULT res = f_mkfs(drv, &opt, workbuf, workbuf_size);
+    res = f_mkfs(drv, &opt, workbuf, workbuf_size);
     free(workbuf);
     if (res != FR_OK) {
         ret = ESP_FAIL;

components/fatfs/vfs/vfs_fat_spiflash.c

@@ -208,7 +208,6 @@ esp_err_t esp_vfs_fat_spiflash_format_rw_wl(const char* base_path, const char* p
 
     wl_handle_t temp_handle = WL_INVALID_HANDLE;
     uint32_t id = FF_VOLUMES;
-    char drv[3] = {0, ':', 0};
 
     bool found = s_get_context_id_by_label(partition_label, &id);
     if (!found) {
@@ -224,8 +223,10 @@ esp_err_t esp_vfs_fat_spiflash_format_rw_wl(const char* base_path, const char* p
     }
 
     //unmount
-    drv[1] = (char)('0' + s_ctx[id]->pdrv);
-    f_mount(0, drv, 0);
+    char drv[3] = {(char)('0' + s_ctx[id]->pdrv), ':', 0};
+    FRESULT fresult = f_mount(0, drv, 0);
+    ESP_RETURN_ON_FALSE(fresult != FR_INVALID_DRIVE, ESP_FAIL, TAG, "f_mount unmount failed (%d) - the logical drive number is invalid", fresult);
+    ESP_GOTO_ON_FALSE(fresult == FR_OK, ESP_FAIL, recycle, TAG, "f_mount unmount failed (%d), go to recycle", fresult);
 
     const size_t workbuf_size = 4096;
     void *workbuf = ff_memalloc(workbuf_size);
@@ -236,7 +237,7 @@ esp_err_t esp_vfs_fat_spiflash_format_rw_wl(const char* base_path, const char* p
     size_t alloc_unit_size = esp_vfs_fat_get_allocation_unit_size(CONFIG_WL_SECTOR_SIZE, s_ctx[id]->mount_config.allocation_unit_size);
     ESP_LOGI(TAG, "Formatting FATFS partition, allocation unit size=%d", alloc_unit_size);
     const MKFS_PARM opt = {(BYTE)(FM_ANY | FM_SFD), 0, 0, 0, alloc_unit_size};
-    FRESULT fresult = f_mkfs(drv, &opt, workbuf, workbuf_size);
+    fresult = f_mkfs(drv, &opt, workbuf, workbuf_size);
     free(workbuf);
     workbuf = NULL;
     ESP_GOTO_ON_FALSE(fresult == FR_OK, ESP_FAIL, mount_back, TAG, "f_mkfs failed (%d)", fresult);