feat(storage/vfs): cleanup path prefix handling

Author: haberturdeur

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;
 
 /**

components/vfs/vfs.c

@@ -395,9 +395,15 @@ static esp_err_t esp_vfs_make_fs_ops(const esp_vfs_t *vfs, esp_vfs_fs_ops_t **mi
     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,16 +473,21 @@ 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);
     if (_vfs == NULL) {
         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)