From 53d5c5f6684ef1d3def260c396eaf7209ba58128 Mon Sep 17 00:00:00 2001 From: Ivan Grokhotkov Date: Thu, 4 May 2017 19:57:11 +0800 Subject: [PATCH] vfs, fatfs: fix support for two FATFS instances (SD and flash) - fix null pointer dereference in VFS when VFS implementations are added and removed in different order - vfs_fat_sdmmc, vfs_fat_spiflash: pass correct drive to mkfs (previously it would always do mkfs in the first drive) - add test case --- components/fatfs/src/vfs_fat_sdmmc.c | 3 +- components/fatfs/src/vfs_fat_spiflash.c | 16 +++++---- components/fatfs/test/test_fatfs_sdmmc.c | 44 ++++++++++++++++++++++++ components/vfs/vfs.c | 3 ++ 4 files changed, 58 insertions(+), 8 deletions(-) diff --git a/components/fatfs/src/vfs_fat_sdmmc.c b/components/fatfs/src/vfs_fat_sdmmc.c index edbc2c12e..cb8324289 100644 --- a/components/fatfs/src/vfs_fat_sdmmc.c +++ b/components/fatfs/src/vfs_fat_sdmmc.c @@ -80,6 +80,7 @@ esp_err_t esp_vfs_fat_sdmmc_mount(const char* base_path, ff_diskio_register_sdmmc(pdrv, s_card); s_pdrv = pdrv; + ESP_LOGD(TAG, "using pdrv=%i", pdrv); char drv[3] = {(char)('0' + pdrv), ':', 0}; // connect FATFS to VFS @@ -109,7 +110,7 @@ esp_err_t esp_vfs_fat_sdmmc_mount(const char* base_path, goto fail; } ESP_LOGW(TAG, "formatting card"); - res = f_mkfs("", FM_ANY, s_card->csd.sector_size, workbuf, workbuf_size); + res = f_mkfs(drv, FM_ANY, s_card->csd.sector_size, workbuf, workbuf_size); if (res != FR_OK) { err = ESP_FAIL; ESP_LOGD(TAG, "f_mkfs failed (%d)", res); diff --git a/components/fatfs/src/vfs_fat_spiflash.c b/components/fatfs/src/vfs_fat_spiflash.c index 10c4781f2..03682e564 100644 --- a/components/fatfs/src/vfs_fat_spiflash.c +++ b/components/fatfs/src/vfs_fat_spiflash.c @@ -45,12 +45,11 @@ esp_err_t esp_vfs_fat_spiflash_mount(const char* base_path, } // connect driver to FATFS BYTE pdrv = 0xFF; - if (ff_diskio_get_drive(&pdrv) != ESP_OK || pdrv == 0xFF) { + if (ff_diskio_get_drive(&pdrv) != ESP_OK) { ESP_LOGD(TAG, "the maximum count of volumes is already mounted"); return ESP_ERR_NO_MEM; } - ESP_LOGD(TAG, "pdrv=%i\n", pdrv); - + ESP_LOGD(TAG, "using pdrv=%i", pdrv); char drv[3] = {(char)('0' + pdrv), ':', 0}; result = ff_diskio_register_wl_partition(pdrv, *wl_handle); @@ -77,7 +76,7 @@ esp_err_t esp_vfs_fat_spiflash_mount(const char* base_path, } workbuf = malloc(workbuf_size); ESP_LOGI(TAG, "Formatting FATFS partition"); - fresult = f_mkfs("", FM_ANY | FM_SFD, workbuf_size, workbuf, workbuf_size); + fresult = f_mkfs(drv, FM_ANY | FM_SFD, workbuf_size, workbuf, workbuf_size); if (fresult != FR_OK) { result = ESP_FAIL; ESP_LOGE(TAG, "f_mkfs failed (%d)", fresult); @@ -103,11 +102,14 @@ fail: esp_err_t esp_vfs_fat_spiflash_unmount(const char *base_path, wl_handle_t wl_handle) { - BYTE s_pdrv = ff_diskio_get_pdrv_wl(wl_handle); - char drv[3] = {(char)('0' + s_pdrv), ':', 0}; + BYTE pdrv = ff_diskio_get_pdrv_wl(wl_handle); + if (pdrv == 0xff) { + return ESP_ERR_INVALID_STATE; + } + char drv[3] = {(char)('0' + pdrv), ':', 0}; f_mount(0, drv, 0); - ff_diskio_unregister(s_pdrv); + ff_diskio_unregister(pdrv); // release partition driver esp_err_t err_drv = wl_unmount(wl_handle); esp_err_t err = esp_vfs_fat_unregister_path(base_path); diff --git a/components/fatfs/test/test_fatfs_sdmmc.c b/components/fatfs/test/test_fatfs_sdmmc.c index f63fb3ef5..bc77d94c8 100644 --- a/components/fatfs/test/test_fatfs_sdmmc.c +++ b/components/fatfs/test/test_fatfs_sdmmc.c @@ -187,3 +187,47 @@ static void speed_test(void* buf, size_t buf_size, size_t file_size, bool write) TEST_ESP_OK(esp_vfs_fat_sdmmc_unmount()); } +TEST_CASE("(SD) mount two FAT partitions, SDMMC and WL, at the same time", "[fatfs][sdcard][ignore]") +{ + esp_vfs_fat_sdmmc_mount_config_t mount_config = { + .format_if_mount_failed = true, + .max_files = 5 + }; + + const char* filename_sd = "/sdcard/sd.txt"; + const char* filename_wl = "/spiflash/wl.txt"; + const char* str_sd = "this is sd\n"; + const char* str_wl = "this is spiflash\n"; + + /* Mount FATFS in SD can WL at the same time. Create a file on each FS */ + wl_handle_t wl_handle = WL_INVALID_HANDLE; + test_setup(); + TEST_ESP_OK(esp_vfs_fat_spiflash_mount("/spiflash", NULL, &mount_config, &wl_handle)); + unlink(filename_sd); + unlink(filename_wl); + test_fatfs_create_file_with_text(filename_sd, str_sd); + test_fatfs_create_file_with_text(filename_wl, str_wl); + TEST_ESP_OK(esp_vfs_fat_spiflash_unmount("/spiflash", wl_handle)); + test_teardown(); + + /* Check that the file "sd.txt" was created on FS in SD, and has the right data */ + test_setup(); + TEST_ASSERT_NULL(fopen(filename_wl, "r")); + FILE* f = fopen(filename_sd, "r"); + TEST_ASSERT_NOT_NULL(f); + char buf[64]; + TEST_ASSERT_NOT_NULL(fgets(buf, sizeof(buf) - 1, f)); + TEST_ASSERT_EQUAL(0, strcmp(buf, str_sd)); + fclose(f); + test_teardown(); + + /* Check that the file "wl.txt" was created on FS in WL, and has the right data */ + TEST_ESP_OK(esp_vfs_fat_spiflash_mount("/spiflash", NULL, &mount_config, &wl_handle)); + TEST_ASSERT_NULL(fopen(filename_sd, "r")); + f = fopen(filename_wl, "r"); + TEST_ASSERT_NOT_NULL(f); + TEST_ASSERT_NOT_NULL(fgets(buf, sizeof(buf) - 1, f)); + TEST_ASSERT_EQUAL(0, strcmp(buf, str_wl)); + fclose(f); + TEST_ESP_OK(esp_vfs_fat_spiflash_unmount("/spiflash", wl_handle)); +} diff --git a/components/vfs/vfs.c b/components/vfs/vfs.c index d80972a53..fff3a93a1 100644 --- a/components/vfs/vfs.c +++ b/components/vfs/vfs.c @@ -125,6 +125,9 @@ static const vfs_entry_t* get_vfs_for_path(const char* path) size_t len = strlen(path); for (size_t i = 0; i < s_vfs_count; ++i) { const vfs_entry_t* vfs = s_vfs[i]; + if (!vfs) { + continue; + } if (len < vfs->path_prefix_len + 1) { // +1 is for the trailing slash after base path continue; }