vfs: add support for nested mount points

Fixes https://github.com/espressif/esp-idf/issues/135

diff --git a/components/vfs/README.rst b/components/vfs/README.rst
index 318004c2500b76ddcfa173d9f3fec52254c71809..d0a34b7de6edce66440b648374b7bde85f73770e 100644 (file)
--- a/components/vfs/README.rst
+++ b/components/vfs/README.rst
@@ -70,15 +70,20 @@ Paths
 
 Each registered FS has a path prefix associated with it. This prefix may be considered a "mount point" of this partition.
 
-Registering mount points which have another mount point as a prefix is not supported and results in undefined behavior. For instance, the following is correct and supported:
+In case when mount points are nested, the mount point with the longest matching path prefix is used when opening the file. For instance, suppose that the following filesystems are registered in VFS:
 
-- FS 1 on /data/fs1
-- FS 2 on /data/fs2
+- FS 1 on /data
+- FS 2 on /data/static
 
-This **will not work** as expected:
+Then:
 
-- FS 1 on /data
-- FS 2 on /data/fs2
+- FS 1 will be used when opening a file called ``/data/log.txt``
+- FS 2 will be used when opening a file called ``/data/static/index.html``
+- Even if ``/index.html"`` doesn't exist in FS 2, FS 1 will *not* be searched for ``/static/index.html``.
+
+As a general rule, mount point names must start with the path separator (``/``) and must contain at least one character after path separator. However an empty mount point name is also supported, and may be used in cases when application needs to provide "fallback" filesystem, or override VFS functionality altogether. Such filesystem will be used if no prefix matches the path given.
+
+VFS does not handle dots (``.``) in path names in any special way. VFS does not treat ``..`` as a reference to the parent directory. I.e. in the above example, using a path ``/data/static/../log.txt`` will not result in a call to FS 1 to open ``/log.txt``. Specific FS drivers (such as FATFS) may handle dots in file names differently.
 
 When opening files, FS driver will only be given relative path to files. For example:
 

diff --git a/components/vfs/test/test_vfs_paths.c b/components/vfs/test/test_vfs_paths.c
index c0a863e188cd2e6d6025125c59492f6eacc4196e..fa00dc4e90e564adb1c53aee8435821c64d8c52b 100644 (file)
--- a/components/vfs/test/test_vfs_paths.c
+++ b/components/vfs/test/test_vfs_paths.c
@@ -173,6 +173,69 @@ TEST_CASE("vfs parses paths correctly", "[vfs]")
     test_not_called(&inst_foo1, "/foo/file1");
     test_opened(&inst_foo1, "/foo1/file1");
     test_not_opened(&inst_foo1, "/foo1/file");
+
+    // Test nested VFS entries
+    dummy_vfs_t inst_foobar = {
+        .match_path = "",
+        .called = false
+    };
+    esp_vfs_t desc_foobar = DUMMY_VFS();
+    TEST_ESP_OK( esp_vfs_register("/foo/bar", &desc_foobar, &inst_foobar) );
+
+    dummy_vfs_t inst_toplevel = {
+        .match_path = "",
+        .called = false
+    };
+    esp_vfs_t desc_toplevel = DUMMY_VFS();
+    TEST_ESP_OK( esp_vfs_register("", &desc_toplevel, &inst_toplevel) );
+
+    inst_foo.match_path = "/bar/file";
+    inst_foobar.match_path = "/file";
+    test_not_called(&inst_foo, "/foo/bar/file");
+    test_opened(&inst_foobar, "/foo/bar/file");
+    test_dir_not_called(&inst_foo, "/foo/bar/file");
+    test_dir_opened(&inst_foobar, "/foo/bar/file");
+    inst_toplevel.match_path = "/tmp/foo";
+    test_opened(&inst_toplevel, "/tmp/foo");
+
     TEST_ESP_OK( esp_vfs_unregister("/foo") );
     TEST_ESP_OK( esp_vfs_unregister("/foo1") );
+    TEST_ESP_OK( esp_vfs_unregister("/foo/bar") );
+    TEST_ESP_OK( esp_vfs_unregister("") );
+}
+
+
+void test_vfs_register(const char* prefix, bool expect_success, int line)
+{
+    dummy_vfs_t inst;
+    esp_vfs_t desc = DUMMY_VFS();
+    esp_err_t err = esp_vfs_register(prefix, &desc, &inst);
+    if (expect_success) {
+        UNITY_TEST_ASSERT_EQUAL_INT(ESP_OK, err, line, "esp_vfs_register should succeed");
+    } else {
+        UNITY_TEST_ASSERT_EQUAL_INT(ESP_ERR_INVALID_ARG,
+                err, line, "esp_vfs_register should fail");
+    }
+    if (err == ESP_OK) {
+        TEST_ESP_OK( esp_vfs_unregister(prefix) );
+    }
+}
+
+#define test_register_ok(prefix) test_vfs_register(prefix, true, __LINE__)
+#define test_register_fail(prefix) test_vfs_register(prefix, false, __LINE__)
+
+TEST_CASE("vfs checks mount point path", "[vfs]")
+{
+    test_register_ok("");
+    test_register_fail("/");
+    test_register_fail("a");
+    test_register_fail("aa");
+    test_register_fail("aaa");
+    test_register_ok("/a");
+    test_register_ok("/aa");
+    test_register_ok("/aaa/bbb");
+    test_register_fail("/aaa/");
+    test_register_fail("/aaa/bbb/");
+    test_register_ok("/23456789012345");
+    test_register_fail("/234567890123456");
 }

diff --git a/components/vfs/vfs.c b/components/vfs/vfs.c
index 6501359bb1dd5a048281cfa478c72f27ac943aa9..226023d5a347cd5de45e78dd4317c688b00b29e8 100644 (file)
--- a/components/vfs/vfs.c
+++ b/components/vfs/vfs.c
@@ -55,10 +55,10 @@ static size_t s_vfs_count = 0;
 esp_err_t esp_vfs_register(const char* base_path, const esp_vfs_t* vfs, void* ctx)
 {
     size_t len = strlen(base_path);
-    if (len < 2 || len > ESP_VFS_PATH_MAX) {
+    if ((len != 0 && len < 2)|| len > ESP_VFS_PATH_MAX) {
         return ESP_ERR_INVALID_ARG;
     }
-    if (base_path[0] != '/' || base_path[len - 1] == '/') {
+    if ((len > 0 && base_path[0] != '/') || base_path[len - 1] == '/') {
         return ESP_ERR_INVALID_ARG;
     }
     vfs_entry_t *entry = (vfs_entry_t*) malloc(sizeof(vfs_entry_t));
@@ -129,6 +129,8 @@ static const char* translate_path(const vfs_entry_t* vfs, const char* src_path)
 
 static const vfs_entry_t* get_vfs_for_path(const char* path)
 {
+    const vfs_entry_t* best_match = NULL;
+    ssize_t best_match_prefix_len = -1;
     size_t len = strlen(path);
     for (size_t i = 0; i < s_vfs_count; ++i) {
         const vfs_entry_t* vfs = s_vfs[i];
@@ -146,9 +148,18 @@ static const vfs_entry_t* get_vfs_for_path(const char* path)
                 path[vfs->path_prefix_len] != '/') {
             continue;
         }
-        return vfs;
+        // Out of all matching path prefixes, select the longest one;
+        // i.e. if "/dev" and "/dev/uart" both match, for "/dev/uart/1" path,
+        // choose "/dev/uart",
+        // This causes all s_vfs_count VFS entries to be scanned when opening
+        // a file by name. This can be optimized by introducing a table for
+        // FS search order, sorted so that longer prefixes are checked first.
+        if (best_match_prefix_len < (ssize_t) vfs->path_prefix_len) {
+            best_match_prefix_len = (ssize_t) vfs->path_prefix_len;
+            best_match = vfs;
+        }
     }
-    return NULL;
+    return best_match;
 }
 
 /*