From 972c6f0caea1d9ec0bb23693859f8c75e18a5fc1 Mon Sep 17 00:00:00 2001 From: me-no-dev Date: Wed, 1 Feb 2017 17:55:25 +0200 Subject: [PATCH] implement review recomendations --- components/fatfs/src/diskio.c | 7 ++-- components/fatfs/src/diskio.h | 14 +++++-- components/fatfs/src/esp_vfs_fat.h | 10 ++++- components/fatfs/src/vfs_fat.c | 56 ++++++++++++++-------------- components/fatfs/src/vfs_fat_sdmmc.c | 29 ++++++++++---- 5 files changed, 72 insertions(+), 44 deletions(-) diff --git a/components/fatfs/src/diskio.c b/components/fatfs/src/diskio.c index 915bb8e2c..c61702f51 100644 --- a/components/fatfs/src/diskio.c +++ b/components/fatfs/src/diskio.c @@ -27,15 +27,16 @@ PARTITION VolToPart[] = { {1, 0} /* Logical drive 1 ==> Physical drive 1, auto detection */ }; -BYTE ff_disk_getpdrv() +esp_err_t ff_diskio_get_drive(BYTE* out_pdrv) { BYTE i; for(i=0; i<_VOLUMES; i++) { if (!s_impls[i]) { - return i; + *out_pdrv = i; + return ESP_OK; } } - return 0xFF; + return ESP_ERR_NOT_FOUND; } void ff_diskio_register(BYTE pdrv, const ff_diskio_impl_t* discio_impl) diff --git a/components/fatfs/src/diskio.h b/components/fatfs/src/diskio.h index 2fd27a864..64d5d5b8d 100644 --- a/components/fatfs/src/diskio.h +++ b/components/fatfs/src/diskio.h @@ -58,16 +58,19 @@ typedef struct { } ff_diskio_impl_t; /** - * Register diskio driver for given drive number. + * Register or unregister diskio driver for given drive number. * * When FATFS library calls one of disk_xxx functions for driver number pdrv, * corresponding function in discio_impl for given pdrv will be called. * * @param pdrv drive number - * @param discio_impl pointer to ff_diskio_impl_t structure with diskio functions + * @param discio_impl pointer to ff_diskio_impl_t structure with diskio functions + * or NULL to unregister and free previously registered drive */ void ff_diskio_register(BYTE pdrv, const ff_diskio_impl_t* discio_impl); +#define ff_diskio_unregister(pdrv_) ff_diskio_register(pdrv_, NULL) + /** * Register SD/MMC diskio driver * @@ -79,9 +82,12 @@ void ff_diskio_register_sdmmc(BYTE pdrv, sdmmc_card_t* card); /** * Get next available drive number * - * @return 0xFF on fail, else the drive number + * @param out_pdrv pointer to the byte to set if successful + * + * @return ESP_OK on success + * ESP_ERR_NOT_FOUND if all drives are attached */ -BYTE ff_disk_getpdrv(); +esp_err_t ff_diskio_get_drive(BYTE* out_pdrv); /* Disk Status Bits (DSTATUS) */ diff --git a/components/fatfs/src/esp_vfs_fat.h b/components/fatfs/src/esp_vfs_fat.h index 6dbef5cfb..9c39198da 100644 --- a/components/fatfs/src/esp_vfs_fat.h +++ b/components/fatfs/src/esp_vfs_fat.h @@ -51,18 +51,24 @@ esp_err_t esp_vfs_fat_register(const char* base_path, const char* fat_drive, * @note FATFS structure returned by esp_vfs_fat_register is destroyed after * this call. Make sure to call f_mount function to unmount it before * calling esp_vfs_fat_unregister. + * This function is left for compatibility and will be changed in + * future versions to accept base_path and replace the method below * @return * - ESP_OK on success * - ESP_ERR_INVALID_STATE if FATFS is not registered in VFS */ -esp_err_t esp_vfs_fat_unregister(); +esp_err_t esp_vfs_fat_unregister() __attribute__((deprecated)); /** * @brief Un-register FATFS from VFS * * @note FATFS structure returned by esp_vfs_fat_register is destroyed after * this call. Make sure to call f_mount function to unmount it before - * calling esp_vfs_fat_unregister. + * calling esp_vfs_fat_unregister_ctx. + * Difference between this function and the one above is that this one + * will release the correct drive, while the one above will release + * the last registered one + * * @param base_path path prefix where FATFS is registered. This is the same * used when esp_vfs_fat_register was called * @return diff --git a/components/fatfs/src/vfs_fat.c b/components/fatfs/src/vfs_fat.c index e85666f1d..10e207482 100644 --- a/components/fatfs/src/vfs_fat.c +++ b/components/fatfs/src/vfs_fat.c @@ -65,12 +65,12 @@ static int vfs_fat_mkdir(void* ctx, const char* name, mode_t mode); static int vfs_fat_rmdir(void* ctx, const char* name); static vfs_fat_ctx_t* s_fat_ctxs[_VOLUMES] = { NULL, NULL }; -//compatibility +//backwards-compatibility with esp_vfs_fat_unregister() static vfs_fat_ctx_t* s_fat_ctx = NULL; -static unsigned char esp_vfs_fat_get_ctx(const char* base_path) +static size_t find_context_index_by_path(const char* base_path) { - for(unsigned char i=0; i<_VOLUMES; i++) { + for(size_t i=0; i<_VOLUMES; i++) { if (s_fat_ctxs[i] && !strcmp(s_fat_ctxs[i]->base_path, base_path)) { return i; } @@ -78,9 +78,9 @@ static unsigned char esp_vfs_fat_get_ctx(const char* base_path) return _VOLUMES; } -static unsigned char esp_vfs_fat_get_empty_ctx() +static size_t find_unused_context_index() { - for(unsigned char i=0; i<_VOLUMES; i++) { + for(size_t i=0; i<_VOLUMES; i++) { if (!s_fat_ctxs[i]) { return i; } @@ -90,12 +90,12 @@ static unsigned char esp_vfs_fat_get_empty_ctx() esp_err_t esp_vfs_fat_register(const char* base_path, const char* fat_drive, size_t max_files, FATFS** out_fs) { - unsigned char ctx = esp_vfs_fat_get_ctx(base_path); + size_t ctx = find_context_index_by_path(base_path); if (ctx < _VOLUMES) { return ESP_ERR_INVALID_STATE; } - ctx = esp_vfs_fat_get_empty_ctx(); + ctx = find_unused_context_index(); if (ctx == _VOLUMES) { return ESP_ERR_NO_MEM; } @@ -127,12 +127,8 @@ esp_err_t esp_vfs_fat_register(const char* base_path, const char* fat_drive, siz return ESP_ERR_NO_MEM; } fat_ctx->max_files = max_files; - - strncpy(fat_ctx->fat_drive, fat_drive, sizeof(fat_ctx->fat_drive) - 1); - fat_ctx->fat_drive[sizeof(fat_ctx->fat_drive) - 1] = 0; - - strncpy(fat_ctx->base_path, base_path, sizeof(fat_ctx->base_path) - 1); - fat_ctx->base_path[sizeof(fat_ctx->base_path) - 1] = 0; + strlcpy(fat_ctx->fat_drive, fat_drive, sizeof(fat_ctx->fat_drive) - 1); + strlcpy(fat_ctx->base_path, base_path, sizeof(fat_ctx->base_path) - 1); esp_err_t err = esp_vfs_register(base_path, &vfs, fat_ctx); if (err != ESP_OK) { @@ -153,7 +149,7 @@ esp_err_t esp_vfs_fat_register(const char* base_path, const char* fat_drive, siz esp_err_t esp_vfs_fat_unregister_ctx(const char* base_path) { - unsigned char ctx = esp_vfs_fat_get_ctx(base_path); + size_t ctx = find_context_index_by_path(base_path); if (ctx == _VOLUMES) { return ESP_ERR_INVALID_STATE; } @@ -249,14 +245,20 @@ static void file_cleanup(vfs_fat_ctx_t* ctx, int fd) memset(&ctx->files[fd], 0, sizeof(FIL)); } -#define vfs_fat_fix_path(ctx, path) \ - char buf_ ## path[strlen(path)+strlen(((vfs_fat_ctx_t*)ctx)->fat_drive)+1]; \ - sprintf(buf_ ## path,"%s%s", ((vfs_fat_ctx_t*)ctx)->fat_drive, path); \ - path = (const char *)buf_ ## path; +static void prepend_drive_to_path(void * ctx, const char * path, const char * path2){ + static char buf[FILENAME_MAX+3]; + static char buf2[FILENAME_MAX+3]; + sprintf(buf, "%s%s", ((vfs_fat_ctx_t*)ctx)->fat_drive, path); + path = (const char *)buf; + if(path2){ + sprintf(buf2, "%s%s", ((vfs_fat_ctx_t*)ctx)->fat_drive, path2); + path2 = (const char *)buf; + } +} static int vfs_fat_open(void* ctx, const char * path, int flags, int mode) { - vfs_fat_fix_path(ctx, path); + prepend_drive_to_path(ctx, path, NULL); ESP_LOGV(TAG, "%s: path=\"%s\", flags=%x, mode=%x", __func__, path, flags, mode); vfs_fat_ctx_t* fat_ctx = (vfs_fat_ctx_t*) ctx; _lock_acquire(&fat_ctx->lock); @@ -366,7 +368,7 @@ static int vfs_fat_fstat(void* ctx, int fd, struct stat * st) static int vfs_fat_stat(void* ctx, const char * path, struct stat * st) { - vfs_fat_fix_path(ctx, path); + prepend_drive_to_path(ctx, path, NULL); FILINFO info; FRESULT res = f_stat(path, &info); if (res != FR_OK) { @@ -396,7 +398,7 @@ static int vfs_fat_stat(void* ctx, const char * path, struct stat * st) static int vfs_fat_unlink(void* ctx, const char *path) { - vfs_fat_fix_path(ctx, path); + prepend_drive_to_path(ctx, path, NULL); FRESULT res = f_unlink(path); if (res != FR_OK) { ESP_LOGD(TAG, "%s: fresult=%d", __func__, res); @@ -408,8 +410,7 @@ static int vfs_fat_unlink(void* ctx, const char *path) static int vfs_fat_link(void* ctx, const char* n1, const char* n2) { - vfs_fat_fix_path(ctx, n1); - vfs_fat_fix_path(ctx, n2); + prepend_drive_to_path(ctx, n1, n2); const size_t copy_buf_size = 4096; void* buf = malloc(copy_buf_size); if (buf == NULL) { @@ -464,8 +465,7 @@ fail1: static int vfs_fat_rename(void* ctx, const char *src, const char *dst) { - vfs_fat_fix_path(ctx, src); - vfs_fat_fix_path(ctx, dst); + prepend_drive_to_path(ctx, src, dst); FRESULT res = f_rename(src, dst); if (res != FR_OK) { ESP_LOGD(TAG, "%s: fresult=%d", __func__, res); @@ -477,7 +477,7 @@ static int vfs_fat_rename(void* ctx, const char *src, const char *dst) static DIR* vfs_fat_opendir(void* ctx, const char* name) { - vfs_fat_fix_path(ctx, name); + prepend_drive_to_path(ctx, name, NULL); vfs_fat_dir_t* fat_dir = calloc(1, sizeof(vfs_fat_dir_t)); if (!fat_dir) { errno = ENOMEM; @@ -582,7 +582,7 @@ static void vfs_fat_seekdir(void* ctx, DIR* pdir, long offset) static int vfs_fat_mkdir(void* ctx, const char* name, mode_t mode) { (void) mode; - vfs_fat_fix_path(ctx, name); + prepend_drive_to_path(ctx, name, NULL); FRESULT res = f_mkdir(name); if (res != FR_OK) { ESP_LOGD(TAG, "%s: fresult=%d", __func__, res); @@ -594,7 +594,7 @@ static int vfs_fat_mkdir(void* ctx, const char* name, mode_t mode) static int vfs_fat_rmdir(void* ctx, const char* name) { - vfs_fat_fix_path(ctx, name); + prepend_drive_to_path(ctx, name, NULL); FRESULT res = f_unlink(name); if (res != FR_OK) { ESP_LOGD(TAG, "%s: fresult=%d", __func__, res); diff --git a/components/fatfs/src/vfs_fat_sdmmc.c b/components/fatfs/src/vfs_fat_sdmmc.c index f5e4c5099..16effdce2 100644 --- a/components/fatfs/src/vfs_fat_sdmmc.c +++ b/components/fatfs/src/vfs_fat_sdmmc.c @@ -13,6 +13,7 @@ // limitations under the License. #include +#include #include "esp_log.h" #include "esp_vfs.h" #include "esp_vfs_fat.h" @@ -23,6 +24,7 @@ static const char* TAG = "vfs_fat_sdmmc"; static sdmmc_card_t* s_card = NULL; static uint8_t s_pdrv = 0; +static char * s_base_path = NULL; esp_err_t esp_vfs_fat_sdmmc_mount(const char* base_path, const sdmmc_host_t* host_config, @@ -36,6 +38,20 @@ esp_err_t esp_vfs_fat_sdmmc_mount(const char* base_path, if (s_card != NULL) { return ESP_ERR_INVALID_STATE; } + + // connect SDMMC driver to FATFS + BYTE pdrv = 0xFF; + if (ff_diskio_get_drive(&pdrv) != ESP_OK || pdrv == 0xFF) { + ESP_LOGD(TAG, "the maximum count of volumes is already mounted"); + return ESP_ERR_NO_MEM; + } + + s_base_path = strdup(base_path); + if(!s_base_path){ + ESP_LOGD(TAG, "could not copy base_path"); + return ESP_ERR_NO_MEM; + } + // enable SDMMC sdmmc_host_init(); @@ -56,12 +72,6 @@ esp_err_t esp_vfs_fat_sdmmc_mount(const char* base_path, *out_card = s_card; } - // connect SDMMC driver to FATFS - BYTE pdrv = ff_disk_getpdrv(); - if (pdrv == 0xFF) { - ESP_LOGD(TAG, "the maximum count of volumes is already mounted"); - goto fail; - } ff_diskio_register_sdmmc(pdrv, s_card); s_pdrv = pdrv; char drv[3] = {(char)('0' + pdrv), ':', 0}; @@ -114,6 +124,7 @@ esp_err_t esp_vfs_fat_sdmmc_mount(const char* base_path, fail: free(workbuf); esp_vfs_fat_unregister_ctx(base_path); + ff_diskio_unregister(pdrv); free(s_card); s_card = NULL; return err; @@ -128,8 +139,12 @@ esp_err_t esp_vfs_fat_sdmmc_unmount() char drv[3] = {(char)('0' + s_pdrv), ':', 0}; f_mount(0, drv, 0); // release SD driver + ff_diskio_unregister(s_pdrv); free(s_card); s_card = NULL; sdmmc_host_deinit(); - return esp_vfs_fat_unregister(); + esp_err_t err = esp_vfs_fat_unregister_ctx(s_base_path); + free(s_base_path); + s_base_path = NULL; + return err; }