Selaa lähdekoodia

Merge branch 'bugfix/spiffs_readdir_recursion' into 'master'

SPIFFS: fix stack overflow in readdir_r due to recursion

See merge request idf/esp-idf!2839
Angus Gratton 7 vuotta sitten
vanhempi
sitoutus
cf593e84d3
2 muutettua tiedostoa jossa 79 lisäystä ja 12 poistoa
  1. 15 12
      components/spiffs/esp_spiffs.c
  2. 64 0
      components/spiffs/test/test_spiffs.c

+ 15 - 12
components/spiffs/esp_spiffs.c

@@ -631,20 +631,23 @@ static int vfs_spiffs_readdir_r(void* ctx, DIR* pdir, struct dirent* entry,
     esp_spiffs_t * efs = (esp_spiffs_t *)ctx;
     vfs_spiffs_dir_t * dir = (vfs_spiffs_dir_t *)pdir;
     struct spiffs_dirent out;
-    if (SPIFFS_readdir(&dir->d, &out) == 0) {
-        errno = spiffs_res_to_errno(SPIFFS_errno(efs->fs));
-        SPIFFS_clearerr(efs->fs);
-        if (!errno) {
-            *out_dirent = NULL;
+    size_t plen;
+    char * item_name;
+    do {
+        if (SPIFFS_readdir(&dir->d, &out) == 0) {
+            errno = spiffs_res_to_errno(SPIFFS_errno(efs->fs));
+            SPIFFS_clearerr(efs->fs);
+            if (!errno) {
+                *out_dirent = NULL;
+            }
+            return errno;
         }
-        return errno;
-    }
-    const char * item_name = (const char *)out.name;
-    size_t plen = strlen(dir->path);
+        item_name = (char *)out.name;
+        plen = strlen(dir->path);
+
+    } while ((plen > 1) && (strncasecmp(dir->path, (const char*)out.name, plen) || out.name[plen] != '/' || !out.name[plen + 1]));
+
     if (plen > 1) {
-        if (strncasecmp(dir->path, (const char *)out.name, plen) || out.name[plen] != '/' || !out.name[plen+1]) {
-            return vfs_spiffs_readdir_r(ctx, pdir, entry, out_dirent);
-        }
         item_name += plen + 1;
     } else if (item_name[0] == '/') {
         item_name++;

+ 64 - 0
components/spiffs/test/test_spiffs.c

@@ -275,6 +275,63 @@ void test_spiffs_opendir_readdir_rewinddir(const char* dir_prefix)
     TEST_ASSERT_EQUAL(0, closedir(dir));
 }
 
+void test_spiffs_readdir_many_files(const char* dir_prefix)
+{
+    const int n_files = 40;
+    const int n_folders = 4;
+    unsigned char file_count[n_files * n_folders];
+    memset(file_count, 0, sizeof(file_count)/sizeof(file_count[0]));
+    char file_name[ESP_VFS_PATH_MAX + CONFIG_SPIFFS_OBJ_NAME_LEN];
+
+    /* clean stale files before the test */
+    DIR* dir = opendir(dir_prefix);
+    if (dir) {
+        while (true) {
+            struct dirent* de = readdir(dir);
+            if (!de) {
+                break;
+            }
+            snprintf(file_name, sizeof(file_name), "%s/%s", dir_prefix, de->d_name);
+            unlink(file_name);
+        }
+    }
+
+    /* create files */
+    for (int d = 0; d < n_folders; ++d) {
+        printf("filling directory %d\n", d);
+        for (int f = 0; f < n_files; ++f) {
+            snprintf(file_name, sizeof(file_name), "%s/%d/%d.txt", dir_prefix, d, f);
+            test_spiffs_create_file_with_text(file_name, file_name);
+        }
+    }
+
+    /* list files */
+    for (int d = 0; d < n_folders; ++d) {
+        printf("listing files in directory %d\n", d);
+        snprintf(file_name, sizeof(file_name), "%s/%d", dir_prefix, d);
+        dir = opendir(file_name);
+        TEST_ASSERT_NOT_NULL(dir);
+        while (true) {
+            struct dirent* de = readdir(dir);
+            if (!de) {
+                break;
+            }
+            int file_id;
+            TEST_ASSERT_EQUAL(1, sscanf(de->d_name, "%d.txt", &file_id));
+            file_count[file_id + d * n_files]++;
+        }
+        closedir(dir);
+    }
+
+    /* check that all created files have been seen */
+    for (int d = 0; d < n_folders; ++d) {
+        printf("checking that all files have been found in directory %d\n", d);
+        for (int f = 0; f < n_files; ++f) {
+            TEST_ASSERT_EQUAL(1, file_count[f + d * n_files]);
+        }
+    }
+}
+
 
 typedef struct {
     const char* filename;
@@ -537,6 +594,13 @@ TEST_CASE("opendir, readdir, rewinddir, seekdir work as expected", "[spiffs]")
     test_teardown();
 }
 
+TEST_CASE("readdir with large number of files", "[spiffs][timeout=15]")
+{
+    test_setup();
+    test_spiffs_readdir_many_files("/spiffs/dir2");
+    test_teardown();
+}
+
 TEST_CASE("multiple tasks can use same volume", "[spiffs]")
 {
     test_setup();