From 67638981ecda4f7b5e5fc73456e50000fc0d638b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tom=C3=A1=C5=A1=20Rohl=C3=ADnek?= Date: Thu, 30 Jan 2025 09:52:37 +0100 Subject: [PATCH] feat(storage/vfs): cleanup path prefix handling --- .../vfs/private_include/esp_vfs_private.h | 10 +-- components/vfs/vfs.c | 64 +++++++++++++------ 2 files changed, 49 insertions(+), 25 deletions(-) diff --git a/components/vfs/private_include/esp_vfs_private.h b/components/vfs/private_include/esp_vfs_private.h index f2f293c400..146e17de44 100644 --- a/components/vfs/private_include/esp_vfs_private.h +++ b/components/vfs/private_include/esp_vfs_private.h @@ -21,11 +21,11 @@ extern "C" { typedef struct vfs_entry_ { int flags; /*!< ESP_VFS_FLAG_CONTEXT_PTR and/or ESP_VFS_FLAG_READONLY_FS or ESP_VFS_FLAG_DEFAULT */ - const esp_vfs_fs_ops_t *vfs; // contains pointers to VFS functions - void *ctx; // optional pointer which can be passed to VFS - int offset; // index of this structure in s_vfs array - size_t path_prefix_len; // micro-optimization to avoid doing extra strlen - const char path_prefix[] __attribute__ ((counted_by (path_prefix_len))); // path prefix mapped to this VFS + const esp_vfs_fs_ops_t *vfs; /*!< contains pointers to VFS functions */ + void *ctx; /*!< optional pointer which can be passed to VFS */ + int offset; /*!< index of this structure in s_vfs array */ + size_t path_prefix_len; /*!< micro-optimization to avoid doing extra strlen, contains length of the string, not the size of path_prefix array */ + const char path_prefix[]; /*!< path prefix mapped to this VFS */ } vfs_entry_t; /** diff --git a/components/vfs/vfs.c b/components/vfs/vfs.c index b756026ce4..bcd5efa3d5 100644 --- a/components/vfs/vfs.c +++ b/components/vfs/vfs.c @@ -395,9 +395,15 @@ fail: return ESP_ERR_NO_MEM; } +static bool is_path_prefix_valid(const char *path, size_t length) { + return (length >= 2) + && (length <= ESP_VFS_PATH_MAX) + && (path[0] == '/') + && (path[length - 1] != '/'); +} + static esp_err_t esp_vfs_register_fs_common( const char *base_path, - size_t base_path_len, const esp_vfs_fs_ops_t *vfs, int flags, void *ctx, @@ -412,13 +418,15 @@ static esp_err_t esp_vfs_register_fs_common( return ESP_ERR_INVALID_ARG; } - if (base_path_len != LEN_PATH_PREFIX_IGNORED) { - /* empty prefix is allowed, "/" is not allowed */ - if ((base_path_len == 1) || (base_path_len > ESP_VFS_PATH_MAX)) { - return ESP_ERR_INVALID_ARG; - } - /* prefix has to start with "/" and not end with "/" */ - if (base_path_len >= 2 && ((base_path[0] != '/') || (base_path[base_path_len - 1] == '/'))) { + size_t base_path_len = 0; + const char *_base_path = ""; + + if (base_path != NULL) { + base_path_len = strlen(base_path); + _base_path = base_path; + + if (base_path_len != 0 && !is_path_prefix_valid(base_path, base_path_len)) { + ESP_LOGE(TAG, "Invalid path prefix"); return ESP_ERR_INVALID_ARG; } } @@ -436,25 +444,20 @@ static esp_err_t esp_vfs_register_fs_common( s_vfs_count++; } - size_t alloc_size = sizeof(vfs_entry_t) - + (base_path_len == LEN_PATH_PREFIX_IGNORED ? 0 : base_path_len + 1); - - vfs_entry_t *entry = (vfs_entry_t*) heap_caps_malloc(alloc_size, VFS_MALLOC_FLAGS); + vfs_entry_t *entry = heap_caps_malloc(sizeof(vfs_entry_t) + base_path_len + 1, VFS_MALLOC_FLAGS); if (entry == NULL) { return ESP_ERR_NO_MEM; } s_vfs[index] = entry; - entry->path_prefix_len = base_path_len; + entry->path_prefix_len = base_path == NULL ? LEN_PATH_PREFIX_IGNORED : base_path_len; entry->vfs = vfs; entry->ctx = ctx; entry->offset = index; entry->flags = flags; - if (base_path_len != LEN_PATH_PREFIX_IGNORED) { - memcpy((char *)(entry->path_prefix), base_path, base_path_len + 1); - } + memcpy((char *)(entry->path_prefix), _base_path, base_path_len + 1); if (vfs_index) { *vfs_index = index; @@ -470,8 +473,13 @@ esp_err_t esp_vfs_register_fs(const char* base_path, const esp_vfs_fs_ops_t* vfs return ESP_ERR_INVALID_ARG; } + if (base_path == NULL) { + ESP_LOGE(TAG, "base_path cannot be null"); + return ESP_ERR_INVALID_ARG; + } + if ((flags & ESP_VFS_FLAG_STATIC)) { - return esp_vfs_register_fs_common(base_path, strlen(base_path), vfs, flags, ctx, NULL); + return esp_vfs_register_fs_common(base_path, vfs, flags, ctx, NULL); } esp_vfs_fs_ops_t *_vfs = esp_vfs_duplicate_fs_ops(vfs); @@ -479,7 +487,7 @@ esp_err_t esp_vfs_register_fs(const char* base_path, const esp_vfs_fs_ops_t* vfs return ESP_ERR_NO_MEM; } - esp_err_t ret = esp_vfs_register_fs_common(base_path, strlen(base_path), _vfs, flags, ctx, NULL); + esp_err_t ret = esp_vfs_register_fs_common(base_path, _vfs, flags, ctx, NULL); if (ret != ESP_OK) { esp_vfs_free_fs_ops(_vfs); return ret; @@ -500,13 +508,29 @@ esp_err_t esp_vfs_register_common(const char* base_path, size_t len, const esp_v return ESP_ERR_INVALID_ARG; } + if (len != LEN_PATH_PREFIX_IGNORED) { + if (base_path == NULL) { + ESP_LOGE(TAG, "Path prefix cannot be NULL"); + return ESP_ERR_INVALID_ARG; + } + + // This is for backwards compatibility + if (strlen(base_path) != len) { + ESP_LOGE(TAG, "Mismatch between real and given path prefix lengths."); + return ESP_ERR_INVALID_ARG; + } + } else { + // New API expects NULL in this case + base_path = NULL; + } + esp_vfs_fs_ops_t *_vfs = NULL; esp_err_t ret = esp_vfs_make_fs_ops(vfs, &_vfs); if (ret != ESP_OK) { return ret; } - ret = esp_vfs_register_fs_common(base_path, len, _vfs, vfs->flags, ctx, vfs_index); + ret = esp_vfs_register_fs_common(base_path, _vfs, vfs->flags, ctx, vfs_index); if (ret != ESP_OK) { esp_vfs_free_fs_ops(_vfs); return ret; @@ -564,7 +588,7 @@ esp_err_t esp_vfs_register_fs_with_id(const esp_vfs_fs_ops_t *vfs, int flags, vo } *vfs_id = -1; - return esp_vfs_register_fs_common("", LEN_PATH_PREFIX_IGNORED, vfs, flags, ctx, vfs_id); + return esp_vfs_register_fs_common(NULL, vfs, flags, ctx, vfs_id); } esp_err_t esp_vfs_register_with_id(const esp_vfs_t *vfs, void *ctx, esp_vfs_id_t *vfs_id)