diff --git a/components/fatfs/host_test/main/test_fatfs.cpp b/components/fatfs/host_test/main/test_fatfs.cpp index 03907424d4..d614bfaba9 100644 --- a/components/fatfs/host_test/main/test_fatfs.cpp +++ b/components/fatfs/host_test/main/test_fatfs.cpp @@ -1,5 +1,5 @@ /* - * SPDX-FileCopyrightText: 2023-2025 Espressif Systems (Shanghai) CO LTD + * SPDX-FileCopyrightText: 2023-2026 Espressif Systems (Shanghai) CO LTD * * SPDX-License-Identifier: Apache-2.0 */ @@ -18,7 +18,13 @@ TEST_CASE("Create volume, open file, write and read back data", "[fatfs]") { BYTE pdrv; FATFS fs; +#if FF_USE_DYN_BUFFER + fs.win = NULL; +#endif FIL file; +#if !FF_FS_TINY && FF_USE_DYN_BUFFER + file.buf = NULL; +#endif UINT bw; const esp_partition_t *partition = esp_partition_find_first(ESP_PARTITION_TYPE_DATA, ESP_PARTITION_SUBTYPE_DATA_FAT, "storage"); @@ -129,12 +135,18 @@ TEST_CASE("Test mounting 2 volumes, writing data and formatting the 2nd one, rea const esp_partition_t *partition0 = NULL; BYTE pdrv0 = UINT8_MAX; FATFS fs0; +#if FF_USE_DYN_BUFFER + fs0.win = NULL; +#endif 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; +#if FF_USE_DYN_BUFFER + fs1.win = NULL; +#endif wl_handle_t wl_handle1 = WL_INVALID_HANDLE; size_t data_size = 10; @@ -150,6 +162,9 @@ TEST_CASE("Test mounting 2 volumes, writing data and formatting the 2nd one, rea // Open file and write data FIL file0; +#if !FF_FS_TINY && FF_USE_DYN_BUFFER + file0.buf = NULL; +#endif UINT bw0; REQUIRE(f_open(&file0, "0:/test0.txt", FA_OPEN_ALWAYS | FA_WRITE) == FR_OK); // Write data @@ -174,6 +189,9 @@ TEST_CASE("Test mounting 2 volumes, writing data and formatting the 2nd one, rea // Open file and write data FIL file1; +#if !FF_FS_TINY && FF_USE_DYN_BUFFER + file1.buf = NULL; +#endif UINT bw1; REQUIRE(f_open(&file1, "1:/test1.txt", FA_OPEN_ALWAYS | FA_WRITE) == FR_OK); // Write data diff --git a/components/fatfs/src/ff.c b/components/fatfs/src/ff.c index 29e5048a4e..b529053447 100644 --- a/components/fatfs/src/ff.c +++ b/components/fatfs/src/ff.c @@ -3513,8 +3513,10 @@ static FRESULT mount_volume ( /* FR_OK(0): successful, !=0: an error occurred */ if (SS(fs) > FF_MAX_SS || SS(fs) < FF_MIN_SS || (SS(fs) & (SS(fs) - 1))) return FR_DISK_ERR; #endif #if FF_USE_DYN_BUFFER - fs->win = ff_memalloc(SS(fs)); /* Allocate memory for sector buffer */ - if (!fs->win) return FR_NOT_ENOUGH_CORE; + if (!fs->win) { + fs->win = ff_memalloc(SS(fs)); /* Allocate memory for sector buffer */ + if (!fs->win) return FR_NOT_ENOUGH_CORE; + } #endif /* Find an FAT volume on the hosting drive */ @@ -3768,8 +3770,10 @@ FRESULT f_mount ( ff_mutex_delete(vol); #endif #if FF_USE_DYN_BUFFER - if (cfs->fs_type) /* Check if the buffer was ever allocated */ + if (cfs->win) { /* Check if the buffer was ever allocated */ ff_memfree(cfs->win); /* Deallocate buffer allocated for the filesystem object */ + cfs->win = NULL; + } #endif cfs->fs_type = 0; /* Invalidate the filesystem object to be unregistered */ } @@ -3955,18 +3959,15 @@ FRESULT f_open ( #if !FF_FS_READONLY #if !FF_FS_TINY #if FF_USE_DYN_BUFFER - fp->buf = NULL; - if (res == FR_OK) { + if (!fp->buf) { fp->buf = ff_memalloc(SS(fs)); if (!fp->buf) { res = FR_NOT_ENOUGH_CORE; /* Not enough memory */ goto fail; } - memset(fp->buf, 0, SS(fs)); /* Clear sector buffer */ } -#else - memset(fp->buf, 0, SS(fs)); /* Clear sector buffer */ #endif + memset(fp->buf, 0, SS(fs)); /* Clear sector buffer */ #endif if ((mode & FA_SEEKEND) && fp->obj.objsize > 0) { /* Seek to end of file if FA_OPEN_APPEND is specified */ DWORD bcs, clst; @@ -4350,15 +4351,17 @@ FRESULT f_close ( #else fp->obj.fs = 0; /* Invalidate file object */ #endif -#if !FF_FS_TINY && FF_USE_DYN_BUFFER - ff_memfree(fp->buf); - fp->buf = NULL; -#endif #if FF_FS_REENTRANT unlock_volume(fs, FR_OK); /* Unlock volume */ #endif } } +#if !FF_FS_TINY && FF_USE_DYN_BUFFER + if (fp->buf) { + ff_memfree(fp->buf); + fp->buf = NULL; + } +#endif return res; } diff --git a/components/fatfs/test_apps/bdl/main/test_fatfs_bdl.c b/components/fatfs/test_apps/bdl/main/test_fatfs_bdl.c index 2cc86df6a1..6752504821 100644 --- a/components/fatfs/test_apps/bdl/main/test_fatfs_bdl.c +++ b/components/fatfs/test_apps/bdl/main/test_fatfs_bdl.c @@ -80,9 +80,15 @@ TEST_CASE("(BDL) diskio register, format, write and read on partition", "[fatfs] TEST_ASSERT_EQUAL(FR_OK, f_mkfs(drv, &opt, work_area, sizeof(work_area))); FATFS fs; +#if FF_USE_DYN_BUFFER + fs.win = NULL; +#endif TEST_ASSERT_EQUAL(FR_OK, f_mount(&fs, drv, 1)); FIL file; +#if !FF_FS_TINY && FF_USE_DYN_BUFFER + file.buf = NULL; +#endif UINT bw; TEST_ASSERT_EQUAL(FR_OK, f_open(&file, "test.txt", FA_OPEN_ALWAYS | FA_READ | FA_WRITE)); diff --git a/components/fatfs/test_apps/dyn_buffers/main/CMakeLists.txt b/components/fatfs/test_apps/dyn_buffers/main/CMakeLists.txt index c3cd1aa78c..15474b0251 100644 --- a/components/fatfs/test_apps/dyn_buffers/main/CMakeLists.txt +++ b/components/fatfs/test_apps/dyn_buffers/main/CMakeLists.txt @@ -1,3 +1,3 @@ idf_component_register(SRCS "test_fatfs_dyn_buffers.c" INCLUDE_DIRS "." - REQUIRES wear_levelling fatfs vfs) + REQUIRES unity wear_levelling fatfs vfs) diff --git a/components/fatfs/test_apps/dyn_buffers/main/test_fatfs_dyn_buffers.c b/components/fatfs/test_apps/dyn_buffers/main/test_fatfs_dyn_buffers.c index c945d97ba9..a0232ee1d0 100644 --- a/components/fatfs/test_apps/dyn_buffers/main/test_fatfs_dyn_buffers.c +++ b/components/fatfs/test_apps/dyn_buffers/main/test_fatfs_dyn_buffers.c @@ -1,5 +1,5 @@ /* - * SPDX-FileCopyrightText: 2025 Espressif Systems (Shanghai) CO LTD + * SPDX-FileCopyrightText: 2025-2026 Espressif Systems (Shanghai) CO LTD * * SPDX-License-Identifier: Unlicense OR CC0-1.0 */ @@ -7,7 +7,10 @@ #include #include #include +#include "unity.h" +#include "unity_test_utils.h" #include "wear_levelling.h" +#include "diskio_wl.h" #include "esp_partition.h" #include "esp_vfs.h" #include "esp_vfs_fat.h" @@ -21,10 +24,19 @@ static const char* TAG = "Test dynamic buffers"; static volatile bool g_alloc_count_enable = false; static volatile int g_buffer_alloc_count = 0; -static esp_vfs_fat_mount_config_t g_mount_config = { - .format_if_mount_failed = true, - .max_files = 5, -}; +// Some resources are lazy allocated, the threshold is left for that case +#define TEST_MEMORY_LEAK_THRESHOLD (1280) + +void setUp(void) +{ + unity_utils_record_free_mem(); +} + +void tearDown(void) +{ + esp_partition_unload_all(); //clean up some of the esp_partition's allocations + unity_utils_evaluate_leaks_direct(TEST_MEMORY_LEAK_THRESHOLD); +} void esp_heap_trace_alloc_hook(void* ptr, size_t size, uint32_t caps) { @@ -42,32 +54,28 @@ void esp_heap_trace_alloc_hook(void* ptr, size_t size, uint32_t caps) } } -void app_main(void) +TEST_CASE("(dyn_buffers) basic mounting and file operations", "[fatfs][dyn_buffers]") { - esp_err_t err = ESP_OK; + esp_vfs_fat_mount_config_t mount_config = { + .format_if_mount_failed = true, + .max_files = 5, + }; wl_handle_t wl_handle; - - err = esp_vfs_fat_spiflash_format_cfg_rw_wl("/spiflash", NULL, &g_mount_config); + esp_vfs_fat_spiflash_format_cfg_rw_wl("/spiflash", NULL, &mount_config); ESP_LOGI(TAG, "Mounting FATFS"); - g_mount_config.format_if_mount_failed = false, + mount_config.format_if_mount_failed = false, g_alloc_count_enable = true; - err = esp_vfs_fat_spiflash_mount_rw_wl("/spiflash", NULL, &g_mount_config, &wl_handle); - if (err != ESP_OK) { - ESP_LOGE(TAG, "FATFS mount failed with error: %d", err); - return; - } + TEST_ESP_OK(esp_vfs_fat_spiflash_mount_rw_wl("/spiflash", NULL, &mount_config, &wl_handle)); ESP_LOGI(TAG, "Mounted"); int fd = open("/spiflash/test.txt", O_RDWR|O_CREAT); - if (fd < 0) { - ESP_LOGE(TAG, "Failed opening file"); - } + TEST_ASSERT(fd >= 0); close(fd); @@ -83,19 +91,67 @@ void app_main(void) , g_buffer_alloc_count); #if CONFIG_FATFS_USE_DYN_BUFFERS - - if (g_buffer_alloc_count != 2) { - ESP_LOGE(TAG, "FATFS buffer should have been allocated once for each context (file and fatfs)"); - return; - } + TEST_ASSERT_MESSAGE(g_buffer_alloc_count == 2, "FATFS buffer should have been allocated once for each context (file and fatfs)"); #else - - if (g_buffer_alloc_count != 0) { - ESP_LOGE(TAG, "FATFS buffer should not have been allocated"); - return; - } - + TEST_ASSERT_MESSAGE(g_buffer_alloc_count == 0, "FATFS buffer should not have been allocated"); #endif ESP_LOGI(TAG, "Done"); } + +TEST_CASE("(dyn_buffers) File system data does not exist, format and remount", "[fatfs][dyn_buffers]") +{ + const char test_par_lab[] = "storage2"; + + const esp_partition_t * part = esp_partition_find_first(ESP_PARTITION_TYPE_DATA, ESP_PARTITION_SUBTYPE_DATA_FAT, test_par_lab); + TEST_ASSERT(part != NULL); + esp_partition_erase_range(part, 0, part->size); + + + esp_vfs_fat_mount_config_t mount_config = { + .format_if_mount_failed = true, + .max_files = 5, + }; + wl_handle_t wl_handle; + TEST_ESP_OK(esp_vfs_fat_spiflash_mount_rw_wl("/spiflash2", test_par_lab, &mount_config, &wl_handle)); + + esp_vfs_fat_spiflash_unmount_rw_wl("/spiflash2", wl_handle); +} + +TEST_CASE("(dyn_buffers) Unmount and then close the file", "[fatfs][dyn_buffers]") +{ + const char test_par_lab[] = "storage2"; + + esp_vfs_fat_mount_config_t mount_config = { + .format_if_mount_failed = true, + .max_files = 5, + }; + wl_handle_t wl_handle; + TEST_ESP_OK(esp_vfs_fat_spiflash_mount_rw_wl("/spiflash2", test_par_lab, &mount_config, &wl_handle)); + BYTE pdrv = ff_diskio_get_pdrv_wl(wl_handle); + char drv[3] = {(char)('0' + pdrv), ':', 0}; + const esp_vfs_fat_conf_t conf = { + .base_path = "/spiflash2", + }; + FATFS *fs = NULL; + esp_vfs_fat_register(&conf, &fs); + TEST_ASSERT_NOT_NULL(fs); + + int fd = open("/spiflash2/test.txt", O_RDWR|O_CREAT); + TEST_ASSERT(fd >= 0); + // Unmount fatfs only + f_mount(0, drv, 0); + close(fd); + + f_mount(fs, drv, 1); + fd = open("/spiflash2/test.txt", O_RDWR|O_CREAT); + TEST_ASSERT(fd >= 0); + // Unmount fatfs and vfs + esp_vfs_fat_spiflash_unmount_rw_wl("/spiflash2", wl_handle); + close(fd); +} + +void app_main(void) +{ + unity_run_menu(); +} diff --git a/components/fatfs/test_apps/dyn_buffers/pytest_fatfs_dyn_buffers.py b/components/fatfs/test_apps/dyn_buffers/pytest_fatfs_dyn_buffers.py index a60ac87e1f..6e8776756a 100644 --- a/components/fatfs/test_apps/dyn_buffers/pytest_fatfs_dyn_buffers.py +++ b/components/fatfs/test_apps/dyn_buffers/pytest_fatfs_dyn_buffers.py @@ -1,4 +1,4 @@ -# SPDX-FileCopyrightText: 2022-2025 Espressif Systems (Shanghai) CO LTD +# SPDX-FileCopyrightText: 2022-2026 Espressif Systems (Shanghai) CO LTD # SPDX-License-Identifier: CC0-1.0 import pytest from pytest_embedded import Dut @@ -15,8 +15,4 @@ from pytest_embedded_idf.utils import idf_parametrize ) @idf_parametrize('target', ['esp32'], indirect=['target']) def test_fatfs_flash_dyn_buffers(config: str, dut: Dut) -> None: - dut.expect('Mounting FATFS') - dut.expect('Mounted') - dut.expect('Unmounting FATFS') - dut.expect('Unmounted') - dut.expect('Done') + dut.run_all_single_board_cases(timeout=30) diff --git a/components/fatfs/vfs/vfs_fat.c b/components/fatfs/vfs/vfs_fat.c index d5fb97a809..11c844c10a 100644 --- a/components/fatfs/vfs/vfs_fat.c +++ b/components/fatfs/vfs/vfs_fat.c @@ -245,6 +245,14 @@ esp_err_t esp_vfs_fat_unregister_path(const char* base_path) return err; } _lock_close(&fat_ctx->lock); + +#if !FF_FS_TINY && FF_USE_DYN_BUFFER + for (size_t i = 0; i < fat_ctx->max_files; ++i) { + if (fat_ctx->files[i].buf != NULL) { + ff_memfree(fat_ctx->files[i].buf); + } + } +#endif free(fat_ctx->flags); free(fat_ctx); s_fat_ctxs[ctx] = NULL;