]> granicus.if.org Git - esp-idf/commitdiff
vfs: fix opendir of a filesystem root directory
authorIvan Grokhotkov <ivan@espressif.com>
Tue, 20 Jun 2017 17:21:14 +0000 (01:21 +0800)
committerIvan Grokhotkov <ivan@espressif.com>
Tue, 20 Jun 2017 17:21:14 +0000 (01:21 +0800)
Previously opendir("/data") would fail if filesystem with "data" prefix
was registered in VFS, while opendir("/data/") would succeed.
This change fixes handling for the former case and adds relevant tests.

components/fatfs/test/test_fatfs_common.c
components/fatfs/test/test_fatfs_common.h
components/fatfs/test/test_fatfs_sdmmc.c
components/fatfs/test/test_fatfs_spiflash.c
components/vfs/test/test_vfs_paths.c [new file with mode: 0644]
components/vfs/vfs.c

index 47dca17e930b85d71e77d8b23e74b6900993ccca..a2f35c3db7e146a9b8309bbaf5e7a3d209097eb4 100644 (file)
@@ -227,6 +227,31 @@ void test_fatfs_mkdir_rmdir(const char* filename_prefix)
     TEST_ASSERT_EQUAL(0, rmdir(name_dir2));
 }
 
+void test_fatfs_can_opendir(const char* path)
+{
+    char name_dir_file[64];
+    const char * file_name = "test_opd.txt";
+    snprintf(name_dir_file, sizeof(name_dir_file), "%s/%s", path, file_name);
+    unlink(name_dir_file);
+    test_fatfs_create_file_with_text(name_dir_file, "test_opendir\n");
+    DIR* dir = opendir(path);
+    TEST_ASSERT_NOT_NULL(dir);
+    bool found = false;
+    while (true) {
+        struct dirent* de = readdir(dir);
+        if (!de) {
+            break;
+        }
+        if (strcasecmp(de->d_name, file_name) == 0) {
+            found = true;
+            break;
+        }
+    }
+    TEST_ASSERT_TRUE(found);
+    TEST_ASSERT_EQUAL(0, closedir(dir));
+    unlink(name_dir_file);
+}
+
 void test_fatfs_opendir_readdir_rewinddir(const char* dir_prefix)
 {
     char name_dir_inner_file[64];
index 84258c1dab4076f7a034cc6a1b846a0d7ab78fde..dcc31e37d44124a2ec3ef03c2c2d6517138535df 100644 (file)
@@ -53,6 +53,8 @@ void test_fatfs_concurrent(const char* filename_prefix);
 
 void test_fatfs_mkdir_rmdir(const char* filename_prefix);
 
+void test_fatfs_can_opendir(const char* path);
+
 void test_fatfs_opendir_readdir_rewinddir(const char* dir_prefix);
 
 void test_fatfs_rw_speed(const char* filename, void* buf, size_t buf_size, size_t file_size, bool write);
index bc77d94c8172b3e630d7cf7d551877da3ecb22a7..0c6d41aff555c0d5f9b2f6e7a2889408649edf1a 100644 (file)
@@ -130,6 +130,13 @@ TEST_CASE("(SD) can create and remove directories", "[fatfs][ignore]")
     test_teardown();
 }
 
+TEST_CASE("(WL) can opendir root directory of FS", "[fatfs][ignore]")
+{
+    test_setup();
+    test_fatfs_can_opendir("/sdcard");
+    test_teardown();
+}
+
 TEST_CASE("(SD) opendir, readdir, rewinddir, seekdir work as expected", "[fatfs][ignore]")
 {
     test_setup();
index 971838db96686c447ab9dc3ef81098cdf0eca7c0..bcc396b93cc80668875b6204a35b1b31da273771 100644 (file)
@@ -125,6 +125,13 @@ TEST_CASE("(WL) can create and remove directories", "[fatfs][wear_levelling]")
     test_teardown();
 }
 
+TEST_CASE("(WL) can opendir root directory of FS", "[fatfs][wear_levelling]")
+{
+    test_setup();
+    test_fatfs_can_opendir("/spiflash");
+    test_teardown();
+}
+
 TEST_CASE("(WL) opendir, readdir, rewinddir, seekdir work as expected", "[fatfs][wear_levelling]")
 {
     test_setup();
diff --git a/components/vfs/test/test_vfs_paths.c b/components/vfs/test/test_vfs_paths.c
new file mode 100644 (file)
index 0000000..033a376
--- /dev/null
@@ -0,0 +1,178 @@
+// Copyright 2015-2017 Espressif Systems (Shanghai) PTE LTD
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+#include <stdbool.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+#include <errno.h>
+#include <sys/fcntl.h>
+#include <sys/dirent.h>
+#include "esp_vfs.h"
+#include "unity.h"
+#include "esp_log.h"
+
+static const char* TAG = "test_vfs_paths";
+
+/* Dummy VFS implementation to check if VFS is called or not with expected path
+ */
+typedef struct {
+    const char* match_path;
+    bool called;
+} dummy_vfs_t;
+
+static int dummy_open(void* ctx, const char * path, int flags, int mode)
+{
+    dummy_vfs_t* dummy = (dummy_vfs_t*) ctx;
+    dummy->called = true;
+    if (strcmp(dummy->match_path, path) == 0) {
+        return 1;
+    }
+    errno = ENOENT;
+    return -1;
+}
+
+static int dummy_close(void* ctx, int fd)
+{
+    dummy_vfs_t* dummy = (dummy_vfs_t*) ctx;
+    dummy->called = true;
+    if (fd == 1) {
+        return 0;
+    }
+    errno = EBADF;
+    return -1;
+}
+
+static DIR* dummy_opendir(void* ctx, const char* path)
+{
+    dummy_vfs_t* dummy = (dummy_vfs_t*) ctx;
+    dummy->called = true;
+    if (strcmp(dummy->match_path, path) == 0) {
+        DIR* result = calloc(1, sizeof(DIR));
+        TEST_ASSERT_NOT_NULL(result);
+        return result;
+    }
+    errno = ENOENT;
+    return NULL;
+}
+
+static int dummy_closedir(void* ctx, DIR* pdir)
+{
+    dummy_vfs_t* dummy = (dummy_vfs_t*) ctx;
+    dummy->called = true;
+    free(pdir);
+    return 0;
+}
+
+/* Initializer for this dummy VFS implementation
+ */
+
+#define DUMMY_VFS() { \
+        .flags = ESP_VFS_FLAG_CONTEXT_PTR, \
+        .open_p = dummy_open, \
+        .close_p = dummy_close, \
+        .opendir_p = dummy_opendir, \
+        .closedir_p = dummy_closedir \
+    }
+
+/* Helper functions to test VFS behavior
+ */
+
+static void test_open(dummy_vfs_t* instance, const char* path,
+        bool should_be_called, bool should_be_opened, int line)
+{
+    const int flags = O_CREAT | O_TRUNC | O_RDWR;
+    instance->called = false;
+    int fd = esp_vfs_open(__getreent(), path, flags, 0);
+    UNITY_TEST_ASSERT_EQUAL_INT(should_be_called, instance->called, line,
+            "should_be_called check failed");
+    if (should_be_called) {
+        if (should_be_opened) {
+            UNITY_TEST_ASSERT(fd >= 0, line, "should be opened");
+        } else {
+            UNITY_TEST_ASSERT(fd < 0, line, "should not be opened");
+        }
+    }
+    esp_vfs_close(__getreent(), fd);
+}
+
+static void test_opendir(dummy_vfs_t* instance, const char* path,
+        bool should_be_called, bool should_be_opened, int line)
+{
+    instance->called = false;
+    DIR* dir = opendir(path);
+    UNITY_TEST_ASSERT_EQUAL_INT(should_be_called, instance->called, line,
+            "should_be_called check failed");
+    if (should_be_called) {
+        if (should_be_opened) {
+            UNITY_TEST_ASSERT(dir != NULL, line, "should be opened");
+        } else {
+            UNITY_TEST_ASSERT(dir == 0, line, "should not be opened");
+        }
+    }
+    if (dir) {
+        closedir(dir);
+    }
+}
+
+/* Helper macros which forward line number to assertion macros inside test_open
+ * and test_opendir
+ */
+
+#define test_opened(instance, path) test_open(instance, path, true, true, __LINE__)
+#define test_not_opened(instance, path) test_open(instance, path, true, false, __LINE__)
+#define test_not_called(instance, path) test_open(instance, path, false, false, __LINE__)
+
+#define test_dir_opened(instance, path) test_opendir(instance, path, true, true, __LINE__)
+#define test_dir_not_opened(instance, path) test_opendir(instance, path, true, false, __LINE__)
+#define test_dir_not_called(instance, path) test_opendir(instance, path, false, false, __LINE__)
+
+
+
+TEST_CASE("vfs parses paths correctly", "[vfs]")
+{
+    dummy_vfs_t inst_foo = {
+            .match_path = "",
+            .called = false
+    };
+    esp_vfs_t desc_foo = DUMMY_VFS();
+    TEST_ESP_OK( esp_vfs_register("/foo", &desc_foo, &inst_foo) );
+
+    dummy_vfs_t inst_foo1 = {
+        .match_path = "",
+        .called = false
+    };
+    esp_vfs_t desc_foo1 = DUMMY_VFS();
+    TEST_ESP_OK( esp_vfs_register("/foo1", &desc_foo1, &inst_foo1) );
+
+    inst_foo.match_path = "/file";
+    test_opened(&inst_foo, "/foo/file");
+    test_not_opened(&inst_foo, "/foo/file1");
+    test_not_called(&inst_foo, "/foo1/file");
+    test_not_called(&inst_foo, "/foo1");
+    test_not_opened(&inst_foo, "/foo");
+    inst_foo.match_path = "/junk";
+    test_dir_opened(&inst_foo, "/foo/junk");
+    inst_foo.match_path = "/";
+    test_dir_opened(&inst_foo, "/foo/");
+    test_dir_opened(&inst_foo, "/foo");
+    test_dir_not_called(&inst_foo1, "/foo");
+    test_dir_not_opened(&inst_foo, "/foo/1");
+    test_dir_not_called(&inst_foo, "/foo1");
+
+    inst_foo1.match_path = "/file1";
+    test_not_called(&inst_foo1, "/foo/file1");
+    test_opened(&inst_foo1, "/foo1/file1");
+    test_not_opened(&inst_foo1, "/foo1/file");
+}
index fff3a93a1dcc8ed1d8417fbcd0f6b77a4ee53545..b656cb22b9d640898a3a4d544cc2cbe483ea1496 100644 (file)
@@ -1,4 +1,4 @@
-// Copyright 2015-2016 Espressif Systems (Shanghai) PTE LTD
+// Copyright 2015-2017 Espressif Systems (Shanghai) PTE LTD
 //
 // Licensed under the Apache License, Version 2.0 (the "License");
 // you may not use this file except in compliance with the License.
@@ -117,6 +117,10 @@ static int translate_fd(const vfs_entry_t* vfs, int fd)
 static const char* translate_path(const vfs_entry_t* vfs, const char* src_path)
 {
     assert(strncmp(src_path, vfs->path_prefix, vfs->path_prefix_len) == 0);
+    if (strlen(src_path) == vfs->path_prefix_len) {
+        // special case when src_path matches the path prefix exactly
+        return "/";
+    }
     return src_path + vfs->path_prefix_len;
 }
 
@@ -128,13 +132,15 @@ static const vfs_entry_t* get_vfs_for_path(const char* path)
         if (!vfs) {
             continue;
         }
-        if (len < vfs->path_prefix_len + 1) {   // +1 is for the trailing slash after base path
-            continue;
-        }
-        if (memcmp(path, vfs->path_prefix, vfs->path_prefix_len) != 0) {  // match prefix
+        // match path prefix
+        if (len < vfs->path_prefix_len ||
+            memcmp(path, vfs->path_prefix, vfs->path_prefix_len) != 0) {
             continue;
         }
-        if (path[vfs->path_prefix_len] != '/') {   // don't match "/data" prefix for "/data1/foo.txt"
+        // if path is not equal to the prefix, expect to see a path separator
+        // i.e. don't match "/data" prefix for "/data1/foo.txt" path
+        if (len > vfs->path_prefix_len &&
+                path[vfs->path_prefix_len] != '/') {
             continue;
         }
         return vfs;