From 03d9707a38e15d3fa41f66486560c2c4c363d969 Mon Sep 17 00:00:00 2001 From: Vincent Dalstra Date: Wed, 18 Feb 2026 14:55:10 +0800 Subject: [PATCH 01/17] Comment out version: "git" This works when managed by the registry, but not as a git submodule, so it needs to be disabled while developing. --- idf_component.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/idf_component.yml b/idf_component.yml index ccfe8da..4fc00c3 100644 --- a/idf_component.yml +++ b/idf_component.yml @@ -1,4 +1,4 @@ -version: "git" +#version: "git" description: A memory based file system for embedded use. url: https://github.com/jkent/ramfs dependencies: From 489335fc8012b02a5e21177151d3030c1d307b47 Mon Sep 17 00:00:00 2001 From: Vincent Dalstra Date: Wed, 18 Feb 2026 15:03:31 +0800 Subject: [PATCH 02/17] Fix incorrect names of struct members --- src/vfs.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/vfs.c b/src/vfs.c index 64241a2..5e51849 100644 --- a/src/vfs.c +++ b/src/vfs.c @@ -137,9 +137,9 @@ static int ramfs_vfs_fstat(void *ctx, int fd, struct stat *st) memset(st, 0, sizeof(*st)); st->st_mode = S_IRWXG | S_IRWXG | S_IRWXO; st->st_size = rst.size; - if (rts.st_mode == RAMFS_ENTRY_TYPE_DIR) { + if (rst.type == RAMFS_ENTRY_TYPE_DIR) { st->st_mode |= S_IFDIR; - } else if (rst.st_mode == RAMFS_ENTRY_TYPE_FILE) { + } else if (rst.type == RAMFS_ENTRY_TYPE_FILE) { st->st_mode |= S_IFREG; } return 0; @@ -160,9 +160,9 @@ static int ramfs_vfs_stat(void *ctx, const char *path, struct stat *st) memset(st, 0, sizeof(*st)); st->st_mode = S_IRWXG | S_IRWXG | S_IRWXO; st->st_size = rst.size; - if (rst.st_mode == RAMFS_ENTRY_TYPE_DIR) { + if (rst.type == RAMFS_ENTRY_TYPE_DIR) { st->st_mode |= S_IFDIR; - } else if (rst.st_mode == RAMFS_ENTRY_TYPE_FILE) { + } else if (rst.type == RAMFS_ENTRY_TYPE_FILE) { st->st_mode |= S_IFREG; } return 0; From f16dc50f7a4bf36151aa4cc0f6f04a40757aa45c Mon Sep 17 00:00:00 2001 From: Vincent Dalstra Date: Thu, 19 Feb 2026 13:07:53 +0800 Subject: [PATCH 03/17] Add log messages --- src/vfs.c | 23 +++++++++++++++++++---- 1 file changed, 19 insertions(+), 4 deletions(-) diff --git a/src/vfs.c b/src/vfs.c index 5e51849..b09127a 100644 --- a/src/vfs.c +++ b/src/vfs.c @@ -6,6 +6,8 @@ #include "ramfs/vfs.h" #include "esp_err.h" +#include "esp_check.h" +#include "esp_log.h" #include "esp_vfs.h" #include @@ -14,6 +16,12 @@ #include #include #include +#include + +static const char* TAG = "ramfs"; + +#undef ESP_LOGV +#define ESP_LOGV ESP_LOGD #ifndef CONFIG_RAMFS_MAX_PARTITIONS @@ -85,6 +93,8 @@ static ssize_t ramfs_vfs_read(void *ctx, int fd, void *data, size_t size) static int ramfs_vfs_open(void *ctx, const char *path, int flags, int mode) { + ESP_LOGV(TAG, "%s: path=\"%s\", flags=%x, mode=%x", __func__, path, flags, mode); + ramfs_vfs_t *vfs = (ramfs_vfs_t *) ctx; int fd; @@ -94,18 +104,22 @@ static int ramfs_vfs_open(void *ctx, const char *path, int flags, int mode) } } if (fd >= vfs->fh_len) { + ESP_LOGE(TAG, "open: no free file descriptors"); + errno = ENFILE; return -1; } ramfs_entry_t *entry = ramfs_get_entry(vfs->fs, path); if (entry == NULL && flags & (O_CREAT | O_TRUNC)) { + ESP_LOGD(TAG, "%s: Create new file: path=\"%s\", flags=%x, mode=%x", __func__, path, flags, mode); entry = ramfs_create(vfs->fs, path, flags); + if (NULL == entry) { + ESP_LOGW(TAG, "Failed to create file: %s", strerror(errno)); + return -1; + } } - if (entry == NULL) { - return -1; - } vfs->fh[fd] = ramfs_open(vfs->fs, entry, flags); return fd; @@ -387,5 +401,6 @@ esp_err_t ramfs_vfs_register(const ramfs_vfs_conf_t *conf) } s_ramfs_vfs[index] = vfs; - return ESP_OK; + + return ESP_OK; } From 2562343003639daad2e053a92332cc77b93a4fa5 Mon Sep 17 00:00:00 2001 From: Vincent Dalstra Date: Thu, 19 Feb 2026 13:21:13 +0800 Subject: [PATCH 04/17] Fix incorrect return code in ramfs_vfs_mkdir() --- src/vfs.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/vfs.c b/src/vfs.c index b09127a..80d4117 100644 --- a/src/vfs.c +++ b/src/vfs.c @@ -285,7 +285,7 @@ static int ramfs_vfs_mkdir(void *ctx, const char *path, mode_t mode) { ramfs_vfs_t *vfs = (ramfs_vfs_t *) ctx; - return ramfs_mkdir(vfs->fs, path) != NULL; + return ramfs_mkdir(vfs->fs, path) != NULL ? 0 : -1; } static int ramfs_vfs_rmdir(void *ctx, const char *path) From aa1114382d86be63ad6d5f6246bfc7a554345e82 Mon Sep 17 00:00:00 2001 From: Vincent Dalstra Date: Thu, 19 Feb 2026 13:28:30 +0800 Subject: [PATCH 05/17] Add log messages --- src/vfs.c | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/src/vfs.c b/src/vfs.c index 80d4117..815b6d9 100644 --- a/src/vfs.c +++ b/src/vfs.c @@ -281,13 +281,19 @@ static void ramfs_vfs_seekdir(void *ctx, DIR *pdir, long offset) ramfs_seekdir(dh->dh, offset); } +__attribute__((nonnull)) static int ramfs_vfs_mkdir(void *ctx, const char *path, mode_t mode) { ramfs_vfs_t *vfs = (ramfs_vfs_t *) ctx; - return ramfs_mkdir(vfs->fs, path) != NULL ? 0 : -1; + if (NULL == ramfs_mkdir(vfs->fs, path)) { + ESP_LOGW(TAG, "cannot make directory '%s': %s", path, strerror(errno)); + return -1; + } + return 0; } +__attribute__((nonnull)) static int ramfs_vfs_rmdir(void *ctx, const char *path) { ramfs_vfs_t *vfs = (ramfs_vfs_t *) ctx; From 8087efb075f97c7d75c7d444f759085de2799e72 Mon Sep 17 00:00:00 2001 From: Vincent Dalstra Date: Thu, 19 Feb 2026 14:22:24 +0800 Subject: [PATCH 06/17] Create a test component --- test/CMakeLists.txt | 21 +++++++ test/README.md | 12 ++++ test/test_ramfs.c | 138 ++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 171 insertions(+) create mode 100644 test/CMakeLists.txt create mode 100644 test/README.md create mode 100644 test/test_ramfs.c diff --git a/test/CMakeLists.txt b/test/CMakeLists.txt new file mode 100644 index 0000000..d791795 --- /dev/null +++ b/test/CMakeLists.txt @@ -0,0 +1,21 @@ +idf_component_register( + SRC_DIRS "." + INCLUDE_DIRS "." + + # Allow us to write tests against 'internal' functions, not just the external API + # INCLUDE_DIRS "../priv_include" + + REQUIRES unity ramfs # Test-framework, and the Module-under-Test + + # Any other components needed by the tests go under here + PRIV_REQUIRES +# code_snippets +# fs_utils + + # Useful for measuring performance +# esp_timer + + # This component sets up eMMC memory/filesystem + PRIV_REQUIRES testfixture_emmc_fatfs + +) diff --git a/test/README.md b/test/README.md new file mode 100644 index 0000000..594c01e --- /dev/null +++ b/test/README.md @@ -0,0 +1,12 @@ +# ramfs/test + +This directory is a 'test component' for use by the esp-idf build system. + +The 'Unity' test-framework is used. A test-application (not included) runs +the tests by calling one of the following: + + unity_run_all_tests() + unity_run_tests_by_tag("[ramfs]", false); + +Note: unity_run-tests_by_tag simply calls strstr() internally, meaning that +it would also work with "ramfs", "fs]" or even "[", but not "[*fs]" \ No newline at end of file diff --git a/test/test_ramfs.c b/test/test_ramfs.c new file mode 100644 index 0000000..792a265 --- /dev/null +++ b/test/test_ramfs.c @@ -0,0 +1,138 @@ +/* Not developed with TDD! + * + * Instead, these are just tests of an existing open-source library. Added after I found + * that the mkdir() function wasn't working! + */ + +// ---- Test framework ---- // +#include "unity.h" + +// ---- Code to be tested ---- // +#include "ramfs/ramfs.h" + +// ---- Additional test fixtures ---- // + +// ---- esp-idf components & system headers ---- // +//#include +#include +#include +#include +#include +#include +#include +#include +#include + +#include "esp_err.h" +#include "esp_check.h" +#include "esp_log.h" + +#include "esp_system.h" // Allows measuring currently available memory + +// ---- Local files (this component) ---- // + +// ---- End of includes ---- // + +// Tag prepended to ESP_LOG messages in this component: can also be used to filter messages at runtime +__attribute__((unused)) +static const char* TAG = "test_ramfs"; + +__attribute__((unused)) +static const char* SANITY = "sanity check for test-fixture"; //!< Marks tests which are to ensure the test-fixture is working correctly (not TDD) + +// ---- private types ---- // + +// ---- private Global variables ---- // + + +// ---- Forward-declared static functions ---- // + + + +// ---- Cleanup functions ---- // + +// This will run before each test! +static void LOCAL_TEST_setUp(void) +{ + errno = 0; +} + +// This will run after each test! +static void LOCAL_TEST_tearDown(void) +{ + + TEST_ASSERT_EQUAL_MESSAGE(0, errno, "If this is expected for this test, then set errno = 0 before calling teardown"); // +} + +// NOTE: These tests were written for an existing libray, and NOT created using TDD! + + +TEST_CASE("Function prototype: register and unregister file system", "[esp_ramfs][TDD-dev]") +{ + LOCAL_TEST_setUp(); + + ramfs_fs_t *fs = ramfs_init(); + assert(fs != NULL); + + ramfs_deinit(fs); + + LOCAL_TEST_tearDown(); +} + +//TEST_CASE("Function prototype: unpack_tarball_to_dir()", "[esp_ramfs][TDD-dev]") +//{ +// LOCAL_TEST_setUp(); +// +// // Function arguments have attribute __nonnull__, so we need to pass it something... +// const char* emptystr = ""; +// unpack_tarball_to_dir_opts_t options = {0}; +// +// // Input file path, output dir path, options +// unpack_tarball_to_dir(emptystr, emptystr, options); +// +// LOCAL_TEST_tearDown(); +//} +// +// +//TEST_CASE("Tar and untar a single file", "[esp_ramfs][TDD-dev]") +//{ +// LOCAL_TEST_setUp(); +// +// const char* tarball_path = LOCAL_TEST_DIR "test.tar"; +// +// // Create file +// const char* input_file = LOCAL_TEST_DIR_INPUT "file.txt"; +// const char* filedata = "abcdef"; +// save_text(input_file, filedata); +// TEST_ASSERT_TRUE_MESSAGE(file_exists(input_file), SANITY); +// +// const char* expected_out_file = LOCAL_TEST_DIR_OUTPUT "file.txt"; +// TEST_ASSERT_FALSE_MESSAGE(file_exists(expected_out_file), SANITY); +// +// // Make tarball +// { +// pack_dir_to_tarball_opts_t options = {0}; +// esp_err_t err = pack_dir_to_tarball(LOCAL_TEST_DIR_INPUT, tarball_path, options); +// TEST_ASSERT_EQUAL(ESP_OK, err); +// } +// +// TEST_ASSERT_TRUE(file_exists(tarball_path)); +// +// // Unpack tarball +// { +// unpack_tarball_to_dir_opts_t options = {0}; +// esp_err_t err = unpack_tarball_to_dir(tarball_path, LOCAL_TEST_DIR_OUTPUT, options); +// TEST_ASSERT_EQUAL(ESP_OK, err); +// } +// +// TEST_ASSERT_TRUE(file_exists(expected_out_file)); +// +// TEST_ASSERT_EQUAL(0, diff_file(input_file, expected_out_file)); +// +// LOCAL_TEST_tearDown(); +//} + + + + + From 6a001180552ef3052c0b404a6c6fca8eeedc38a4 Mon Sep 17 00:00:00 2001 From: Vincent Dalstra Date: Thu, 19 Feb 2026 14:49:07 +0800 Subject: [PATCH 07/17] Add a ramfs_vfs_unregister() function This makes Unit-tests better, because they can allocate/cleanup every single time. However, it fails with ESP_ERR_INVALID_STATE each time. Could this be a clue to why things are going wrong? --- include/ramfs/vfs.h | 7 +++++++ src/vfs.c | 6 ++++++ test/test_ramfs.c | 14 +++++++++++++- 3 files changed, 26 insertions(+), 1 deletion(-) diff --git a/include/ramfs/vfs.h b/include/ramfs/vfs.h index 20b9f6f..bed6597 100644 --- a/include/ramfs/vfs.h +++ b/include/ramfs/vfs.h @@ -30,6 +30,13 @@ typedef struct ramfs_vfs_conf_t { */ esp_err_t ramfs_vfs_register(const ramfs_vfs_conf_t *conf); +/** + * \brief Unmount a ramfs handle that was previously mounted with ramfs_vfs_register + * \param[in] conf vfs configuration + * \return ESP_OK if successful, ESP_ERR_INVALID_STATE if the vfs is not mounted + */ +esp_err_t ramfs_vfs_unregister(const ramfs_vfs_conf_t *conf); + #ifdef __cplusplus } /* extern "C" */ #endif diff --git a/src/vfs.c b/src/vfs.c index 815b6d9..1823733 100644 --- a/src/vfs.c +++ b/src/vfs.c @@ -410,3 +410,9 @@ esp_err_t ramfs_vfs_register(const ramfs_vfs_conf_t *conf) return ESP_OK; } + +esp_err_t ramfs_vfs_unregister(const ramfs_vfs_conf_t *conf) +{ + return esp_vfs_unregister(conf->base_path); +} + diff --git a/test/test_ramfs.c b/test/test_ramfs.c index 792a265..0c60a94 100644 --- a/test/test_ramfs.c +++ b/test/test_ramfs.c @@ -5,10 +5,11 @@ */ // ---- Test framework ---- // +#include "freertos/projdefs.h" #include "unity.h" // ---- Code to be tested ---- // -#include "ramfs/ramfs.h" +#include "ramfs/vfs.h" // ---- Additional test fixtures ---- // @@ -74,6 +75,17 @@ TEST_CASE("Function prototype: register and unregister file system", "[esp_ramfs ramfs_fs_t *fs = ramfs_init(); assert(fs != NULL); + ramfs_vfs_conf_t ramfs_vfs_conf = { + .base_path = "/ramfs", + .fs = fs, + .max_files = 5, + }; + ESP_ERROR_CHECK(ramfs_vfs_register(&ramfs_vfs_conf)); + + // Mounted! + + ESP_ERROR_CHECK(ramfs_vfs_unregister(&ramfs_vfs_conf)); + ramfs_deinit(fs); LOCAL_TEST_tearDown(); From c2bfd020debb836ed0fe740cdc058175e5565afd Mon Sep 17 00:00:00 2001 From: Vincent Dalstra Date: Thu, 19 Feb 2026 14:58:54 +0800 Subject: [PATCH 08/17] More logging functions --- src/vfs.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/vfs.c b/src/vfs.c index 1823733..7d2c0ad 100644 --- a/src/vfs.c +++ b/src/vfs.c @@ -355,6 +355,8 @@ static int ramfs_vfs_ftruncate(void *ctx, int fd, off_t length) esp_err_t ramfs_vfs_register(const ramfs_vfs_conf_t *conf) { + ESP_LOGV(TAG, "%s: path=\"%s\", fs=0x%p, max_files=%d", __func__, conf->base_path, conf->fs, conf->max_files); + assert(conf != NULL); assert(conf->fs != NULL); assert(conf->base_path != NULL); @@ -400,6 +402,8 @@ esp_err_t ramfs_vfs_register(const ramfs_vfs_conf_t *conf) strlcpy(vfs->base_path, conf->base_path, sizeof(*vfs->base_path)); vfs->fh_len = conf->max_files; + ESP_LOGD(TAG, "register ramfs_vs, index=%d, base_path='%s'", index, vfs->base_path); + esp_err_t err = esp_vfs_register(vfs->base_path, &funcs, vfs); if (err != ESP_OK) { free(vfs); From be0d796f7f60170658b3b689d5d3f57ed2732c82 Mon Sep 17 00:00:00 2001 From: Vincent Dalstra Date: Thu, 19 Feb 2026 14:59:06 +0800 Subject: [PATCH 09/17] Found the bug! Bad sizeof operator. A classic c bug! I added a runtime check that will catch that error if it were to resurface again. It's also preferable to the previous behaviour of 'truncate silently if the path is too long'. Give people errors when they do things wrong! --- src/vfs.c | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/src/vfs.c b/src/vfs.c index 7d2c0ad..64b64ea 100644 --- a/src/vfs.c +++ b/src/vfs.c @@ -399,9 +399,16 @@ esp_err_t ramfs_vfs_register(const ramfs_vfs_conf_t *conf) } vfs->fs = conf->fs; - strlcpy(vfs->base_path, conf->base_path, sizeof(*vfs->base_path)); + strlcpy(vfs->base_path, conf->base_path, sizeof(vfs->base_path)); vfs->fh_len = conf->max_files; + // Check that the whole path was copied + if (strcmp(vfs->base_path, conf->base_path) != 0) { + ESP_LOGE(TAG, "base path '%s' is too long!", conf->base_path); // If not, check the strlcpy() length parameter for bugs + free(vfs); + return ESP_ERR_INVALID_ARG; + } + ESP_LOGD(TAG, "register ramfs_vs, index=%d, base_path='%s'", index, vfs->base_path); esp_err_t err = esp_vfs_register(vfs->base_path, &funcs, vfs); From 6360b16d1ec10bcc092d5d5249df72406121686a Mon Sep 17 00:00:00 2001 From: Vincent Dalstra Date: Thu, 19 Feb 2026 15:39:18 +0800 Subject: [PATCH 10/17] TDD: Proeprly clear internal data during ramfs_vfs_unregister() --- src/vfs.c | 23 +++++++++++ test/test_ramfs.c | 97 ++++++++++++++++------------------------------- 2 files changed, 55 insertions(+), 65 deletions(-) diff --git a/src/vfs.c b/src/vfs.c index 64b64ea..a3c0493 100644 --- a/src/vfs.c +++ b/src/vfs.c @@ -58,6 +58,21 @@ static esp_err_t ramfs_get_empty(int *index) return ESP_ERR_NOT_FOUND; } +static esp_err_t ramfs_get_index_from_path(int *index, const char* base_path) +{ + int i; + + for (i = 0; i < CONFIG_RAMFS_MAX_PARTITIONS; i++) { + if (s_ramfs_vfs[i] != NULL) { + if (0 == strcmp(base_path, s_ramfs_vfs[i]->base_path)) { + *index = i; + return ESP_OK; + } + } + } + return ESP_ERR_NOT_FOUND; +} + static ssize_t ramfs_vfs_write(void *ctx, int fd, const void *data, size_t size) { ramfs_vfs_t *vfs = (ramfs_vfs_t *) ctx; @@ -424,6 +439,14 @@ esp_err_t ramfs_vfs_register(const ramfs_vfs_conf_t *conf) esp_err_t ramfs_vfs_unregister(const ramfs_vfs_conf_t *conf) { + int i; + ESP_RETURN_ON_ERROR( + ramfs_get_index_from_path(&i, conf->base_path), + TAG, "no ramfs found for '%s'", conf->base_path + ); + + s_ramfs_vfs[i] = NULL; + return esp_vfs_unregister(conf->base_path); } diff --git a/test/test_ramfs.c b/test/test_ramfs.c index 0c60a94..06d34da 100644 --- a/test/test_ramfs.c +++ b/test/test_ramfs.c @@ -45,6 +45,13 @@ static const char* SANITY = "sanity check for test-fixture"; //!< Marks t // ---- private Global variables ---- // +#define MOUNT_POINT "/ramfs" + +ramfs_vfs_conf_t ramfs_vfs_conf = { + .base_path = MOUNT_POINT, + .fs = NULL, + .max_files = 5, +}; // ---- Forward-declared static functions ---- // @@ -55,6 +62,12 @@ static const char* SANITY = "sanity check for test-fixture"; //!< Marks t // This will run before each test! static void LOCAL_TEST_setUp(void) { + // Create and mount + ramfs_vfs_conf.fs = ramfs_init(); + assert(ramfs_vfs_conf.fs != NULL); + ESP_ERROR_CHECK(ramfs_vfs_register(&ramfs_vfs_conf)); + // + errno = 0; } @@ -63,87 +76,41 @@ static void LOCAL_TEST_tearDown(void) { TEST_ASSERT_EQUAL_MESSAGE(0, errno, "If this is expected for this test, then set errno = 0 before calling teardown"); // + + // Unmount and destroy + ESP_ERROR_CHECK(ramfs_vfs_unregister(&ramfs_vfs_conf)); + ramfs_deinit(ramfs_vfs_conf.fs); + ramfs_vfs_conf.fs = NULL; } // NOTE: These tests were written for an existing libray, and NOT created using TDD! -TEST_CASE("Function prototype: register and unregister file system", "[esp_ramfs][TDD-dev]") +TEST_CASE("Register and Unregister file system", "[esp_ramfs][TDD-dev]") { LOCAL_TEST_setUp(); - ramfs_fs_t *fs = ramfs_init(); - assert(fs != NULL); + // This test was essentially moved into setUp/tearDown - ramfs_vfs_conf_t ramfs_vfs_conf = { - .base_path = "/ramfs", - .fs = fs, - .max_files = 5, - }; - ESP_ERROR_CHECK(ramfs_vfs_register(&ramfs_vfs_conf)); + LOCAL_TEST_tearDown(); +} + +TEST_CASE("Create a file in the root directory", "[esp_ramfs][TDD-dev]") +{ + LOCAL_TEST_setUp(); - // Mounted! + const char* filepath = MOUNT_POINT "/file.txt"; - ESP_ERROR_CHECK(ramfs_vfs_unregister(&ramfs_vfs_conf)); + FILE* fd = fopen(filepath, "w"); + TEST_ASSERT_NOT_NULL(fd); - ramfs_deinit(fs); + int ret = fclose(fd); + TEST_ASSERT_EQUAL(0, ret); + LOCAL_TEST_tearDown(); } -//TEST_CASE("Function prototype: unpack_tarball_to_dir()", "[esp_ramfs][TDD-dev]") -//{ -// LOCAL_TEST_setUp(); -// -// // Function arguments have attribute __nonnull__, so we need to pass it something... -// const char* emptystr = ""; -// unpack_tarball_to_dir_opts_t options = {0}; -// -// // Input file path, output dir path, options -// unpack_tarball_to_dir(emptystr, emptystr, options); -// -// LOCAL_TEST_tearDown(); -//} -// -// -//TEST_CASE("Tar and untar a single file", "[esp_ramfs][TDD-dev]") -//{ -// LOCAL_TEST_setUp(); -// -// const char* tarball_path = LOCAL_TEST_DIR "test.tar"; -// -// // Create file -// const char* input_file = LOCAL_TEST_DIR_INPUT "file.txt"; -// const char* filedata = "abcdef"; -// save_text(input_file, filedata); -// TEST_ASSERT_TRUE_MESSAGE(file_exists(input_file), SANITY); -// -// const char* expected_out_file = LOCAL_TEST_DIR_OUTPUT "file.txt"; -// TEST_ASSERT_FALSE_MESSAGE(file_exists(expected_out_file), SANITY); -// -// // Make tarball -// { -// pack_dir_to_tarball_opts_t options = {0}; -// esp_err_t err = pack_dir_to_tarball(LOCAL_TEST_DIR_INPUT, tarball_path, options); -// TEST_ASSERT_EQUAL(ESP_OK, err); -// } -// -// TEST_ASSERT_TRUE(file_exists(tarball_path)); -// -// // Unpack tarball -// { -// unpack_tarball_to_dir_opts_t options = {0}; -// esp_err_t err = unpack_tarball_to_dir(tarball_path, LOCAL_TEST_DIR_OUTPUT, options); -// TEST_ASSERT_EQUAL(ESP_OK, err); -// } -// -// TEST_ASSERT_TRUE(file_exists(expected_out_file)); -// -// TEST_ASSERT_EQUAL(0, diff_file(input_file, expected_out_file)); -// -// LOCAL_TEST_tearDown(); -//} - From a53f97465d646a8390067817dd7248707b5b10ca Mon Sep 17 00:00:00 2001 From: Vincent Dalstra Date: Thu, 19 Feb 2026 15:53:56 +0800 Subject: [PATCH 11/17] TDD: Fix bug when opening non-existent files I had introduced this bug while adding an error message. I thought I was helping, but did not consider the && in the if() statement. Split the if(&&) statement into two if() statements, to make the control flow clearer - and prevent the above from happening. --- src/vfs.c | 20 ++++++++---- test/test_ramfs.c | 82 ++++++++++++++++++++++++++++++++++++++++++----- 2 files changed, 88 insertions(+), 14 deletions(-) diff --git a/src/vfs.c b/src/vfs.c index a3c0493..c824d38 100644 --- a/src/vfs.c +++ b/src/vfs.c @@ -126,14 +126,22 @@ static int ramfs_vfs_open(void *ctx, const char *path, int flags, int mode) ramfs_entry_t *entry = ramfs_get_entry(vfs->fs, path); - if (entry == NULL && flags & (O_CREAT | O_TRUNC)) { - ESP_LOGD(TAG, "%s: Create new file: path=\"%s\", flags=%x, mode=%x", __func__, path, flags, mode); - entry = ramfs_create(vfs->fs, path, flags); - if (NULL == entry) { - ESP_LOGW(TAG, "Failed to create file: %s", strerror(errno)); + if (NULL == entry) { + if (flags & (O_CREAT | O_TRUNC)) { + ESP_LOGD(TAG, "%s: Create new file: path=\"%s\", flags=%x, mode=%x", __func__, path, flags, mode); + entry = ramfs_create(vfs->fs, path, flags); + if (NULL == entry) { + ESP_LOGW(TAG, "Failed to create new file at '%s': %s", path, strerror(errno)); + return -1; + } + } else { + ESP_LOGW(TAG, "No file at '%s': %s", path, strerror(errno)); return -1; } } + assert(entry != NULL); + + ESP_LOGV(TAG, "%s: found entry for '%s', entry=%p", __func__, path, entry); vfs->fh[fd] = ramfs_open(vfs->fs, entry, flags); @@ -370,7 +378,7 @@ static int ramfs_vfs_ftruncate(void *ctx, int fd, off_t length) esp_err_t ramfs_vfs_register(const ramfs_vfs_conf_t *conf) { - ESP_LOGV(TAG, "%s: path=\"%s\", fs=0x%p, max_files=%d", __func__, conf->base_path, conf->fs, conf->max_files); + ESP_LOGV(TAG, "%s: path=\"%s\", fs=%p, max_files=%d", __func__, conf->base_path, conf->fs, conf->max_files); assert(conf != NULL); assert(conf->fs != NULL); diff --git a/test/test_ramfs.c b/test/test_ramfs.c index 06d34da..e82c377 100644 --- a/test/test_ramfs.c +++ b/test/test_ramfs.c @@ -21,6 +21,7 @@ #include #include #include +#include #include #include @@ -56,6 +57,27 @@ ramfs_vfs_conf_t ramfs_vfs_conf = { // ---- Forward-declared static functions ---- // +static bool file_exists(const char* path) +{ + FILE* fd = fopen(path, "r"); + if (NULL == fd) { + return false; + } + fclose(fd); + return true; +} + +static bool dir_exists(const char* path) +{ + DIR* dir = opendir(path); + if (dir == NULL) { + return false; + } + closedir(dir); + return true; +} + + // ---- Cleanup functions ---- // @@ -74,16 +96,14 @@ static void LOCAL_TEST_setUp(void) // This will run after each test! static void LOCAL_TEST_tearDown(void) { - - TEST_ASSERT_EQUAL_MESSAGE(0, errno, "If this is expected for this test, then set errno = 0 before calling teardown"); // - // Unmount and destroy ESP_ERROR_CHECK(ramfs_vfs_unregister(&ramfs_vfs_conf)); ramfs_deinit(ramfs_vfs_conf.fs); ramfs_vfs_conf.fs = NULL; } -// NOTE: These tests were written for an existing libray, and NOT created using TDD! +// NOTE: These tests were written for an existing libray, not originally created with TDD. +// There were quite a few bugs though, so much of the below is TDD and can be relied upon TEST_CASE("Register and Unregister file system", "[esp_ramfs][TDD-dev]") @@ -101,17 +121,63 @@ TEST_CASE("Create a file in the root directory", "[esp_ramfs][TDD-dev]") const char* filepath = MOUNT_POINT "/file.txt"; - FILE* fd = fopen(filepath, "w"); - TEST_ASSERT_NOT_NULL(fd); + // Shouldn't be there yet + TEST_ASSERT_FALSE(file_exists(filepath)); - int ret = fclose(fd); - TEST_ASSERT_EQUAL(0, ret); + // Open for writing, creates the file + { + FILE* fd = fopen(filepath, "w"); + TEST_ASSERT_NOT_NULL(fd); + int ret = fclose(fd); + TEST_ASSERT_EQUAL(0, ret); + } + + TEST_ASSERT_TRUE(file_exists(filepath)); + + LOCAL_TEST_tearDown(); +} + + +// ---- Tests below never failed, and so are marked with the [not-TDD] tag + +TEST_CASE("Create a directory", "[esp_ramfs][not-TDD]") +{ + LOCAL_TEST_setUp(); + + const char* dirpath = MOUNT_POINT "/dir"; + + // Shouldn't be there yet + TEST_ASSERT_FALSE(dir_exists(dirpath)); + + // Create the directory + { + mkdir(dirpath, 0755); + } + + TEST_ASSERT_TRUE(dir_exists(dirpath)); LOCAL_TEST_tearDown(); } +TEST_CASE("Create a file in a directory", "[esp_ramfs][not-TDD]") +{ + LOCAL_TEST_setUp(); + + const char* dirpath = MOUNT_POINT "/dir"; + + // Shouldn't be there yet + TEST_ASSERT_FALSE(dir_exists(dirpath)); + + // Create the directory + { + mkdir(dirpath, 0755); + } + TEST_ASSERT_TRUE(dir_exists(dirpath)); + + LOCAL_TEST_tearDown(); +} From a88207d7b6296a59fd193a331a8b2c855346c995 Mon Sep 17 00:00:00 2001 From: Vincent Dalstra Date: Tue, 3 Mar 2026 15:25:51 +0800 Subject: [PATCH 12/17] Enable GCC program analyzer --- test/CMakeLists.txt | 3 +++ 1 file changed, 3 insertions(+) diff --git a/test/CMakeLists.txt b/test/CMakeLists.txt index d791795..98bdbd7 100644 --- a/test/CMakeLists.txt +++ b/test/CMakeLists.txt @@ -19,3 +19,6 @@ idf_component_register( PRIV_REQUIRES testfixture_emmc_fatfs ) + +target_compile_options(${COMPONENT_LIB} PRIVATE -Werror) # Doesn't work? +target_compile_options(${COMPONENT_LIB} PRIVATE -fanalyzer) \ No newline at end of file From 11b97f438e14710390002ba780466bcafbc57ea9 Mon Sep 17 00:00:00 2001 From: Vincent Dalstra Date: Thu, 19 Feb 2026 16:29:20 +0800 Subject: [PATCH 13/17] TDD: Fix various bugs and non-compliant behaviour When using this component to substitute for the esp-idf's fatfs component, I discovered various bugs and non-POSIX-compliant behaviour that resulted in unit tests failing. - Not handling trailing '/' in paths properly. - Incorrect results from readdir() when the directory is modified. - Setting errno when an operatioon succeeds. - Setting ENFILE instead of EISDIR during unlink(). These errors were fixed using Test-Driven-Development, meaning there is excellent test coverage. Some of the original commit messages are kept below: --- POSIX compliant readdir() behaviour when files are deleted When a file is added or deleted from a directory after opendir(), POSIX leaves unspecified whether the file will be returned from a subsequent readdir() call. That only applies to the the files that were added and removed, though. All other files should be returned if-and-only-if they were not already returned. However, because the array of entries is shifted back when an element is removed, removing a file behind the iterator would result in a file being skipped! To reliably detect changes, a copy of the directory handle is made during opendir(), and entry pointer compared during each iteration to see if anything was removed. Use correct errno value for unlink() Previously, when unlink() was called on a directory it would return ENFILE, which I think was intended to represent "not a file", but which really means "run out of file descriptors". Now returns the correct value of EISDIR. Check for errors during ramfs_rmtree() Ignore trailing slash's in opendir(), mkdir() etc... Fix opendir() returning NULL for empty directories. Fix erroneously setting errno in soem situations. Do not allow trailing slash when path refers to a file --- include/ramfs/ramfs.h | 3 +- src/ramfs_vector.c | 99 ++++++++-- src/vfs.c | 137 ++++++++++--- test/test_ramfs.c | 443 ++++++++++++++++++++++++++++++++++++++++-- 4 files changed, 630 insertions(+), 52 deletions(-) diff --git a/include/ramfs/ramfs.h b/include/ramfs/ramfs.h index b63df22..2fd277a 100644 --- a/include/ramfs/ramfs.h +++ b/include/ramfs/ramfs.h @@ -268,8 +268,9 @@ int ramfs_rmdir(ramfs_entry_t *entry); /** * \brief Delete and free a directory tree * \param entry root entry to remove + * \return 0 on success, -1 on error */ -void ramfs_rmtree(ramfs_entry_t *entry); +int ramfs_rmtree(ramfs_entry_t *entry); #ifdef __cplusplus } /* extern "C" */ diff --git a/src/ramfs_vector.c b/src/ramfs_vector.c index 4b5728b..7f93d99 100644 --- a/src/ramfs_vector.c +++ b/src/ramfs_vector.c @@ -43,6 +43,9 @@ typedef struct ramfs_dh_t { ramfs_fs_t *fs; ramfs_dir_t *dir; size_t loc; + ramfs_dir_t cached_dir; + size_t cached_loc; + int go_slow; // boolean } ramfs_dh_t; typedef struct ramfs_fh_t { @@ -72,7 +75,7 @@ static ssize_t find_entry(ramfs_entry_t *dir, const char *name) } } - errno = ENOENT; +// errno = ENOENT; // Let the caller decide if this is an error or not. return -(last + 2); } @@ -132,6 +135,7 @@ static ramfs_entry_t *remove(ramfs_entry_t *entry) { ssize_t i = find_entry(&entry->parent->entry, entry->name); if (i < 0) { + errno = ENOENT; return NULL; } @@ -153,7 +157,9 @@ void ramfs_deinit(ramfs_fs_t *fs) { assert(fs != NULL); - ramfs_rmtree(&fs->root.entry); + int ret = ramfs_rmtree(&fs->root.entry); + assert(ret == 0); + free(fs); } @@ -179,6 +185,7 @@ ramfs_entry_t *ramfs_get_parent(ramfs_fs_t *fs, const char *path) ssize_t i = find_entry(&dir->entry, key); free(key); if (i < 0) { + errno = ENOENT; return NULL; } if (!ramfs_is_dir(dir->children[i])) { @@ -222,7 +229,8 @@ ramfs_entry_t *ramfs_get_entry(ramfs_fs_t *fs, const char *path) ssize_t i = find_entry(&parent->entry, key); if (i < 0) { - return NULL; + errno = ENOENT; + return NULL; } return parent->children[i]; @@ -321,7 +329,7 @@ ramfs_entry_t *ramfs_create(ramfs_fs_t *fs, const char *path, int flags) errno = EEXIST; return NULL; } - i = -i - 1; + i = -i - 1; // Pretty sure this is always the last index + 1 i.e. 'insert after the last element' if (strlen(name) == 0 || strchr(name, '/')) { errno = EINVAL; @@ -503,11 +511,13 @@ int ramfs_unlink(ramfs_entry_t *entry) { assert(entry != NULL); - if (entry->type != RAMFS_ENTRY_TYPE_FILE) { - errno = ENFILE; + if (entry->type == RAMFS_ENTRY_TYPE_DIR) { + errno = EISDIR; return -1; } + assert(entry->type == RAMFS_ENTRY_TYPE_FILE); + ramfs_file_t* file = (ramfs_file_t*) entry; if (remove(entry) == NULL) { @@ -555,7 +565,8 @@ int ramfs_rename(ramfs_fs_t *fs, const char *src, const char *dst) } ssize_t src_index = find_entry(&src_parent->entry, name); if (src_index < 0) { - return -1; + errno = ENOENT; + return -1; } len = strlen(dst); @@ -600,8 +611,24 @@ ramfs_dh_t *ramfs_opendir(ramfs_fs_t *fs, const ramfs_entry_t *entry) } ramfs_dh_t *dh = calloc(1, sizeof(*dh)); + if (NULL == dh) { + return NULL; + } dh->fs = fs; dh->dir = (ramfs_dir_t *) entry; + + // Make a copy of the directory's current state + dh->cached_dir = *dh->dir; + dh->cached_dir.children = calloc(dh->cached_dir.children_len, sizeof(ramfs_entry_t*)); + if (NULL == dh->cached_dir.children) { + // NULL may be returned when there are no children + if (dh->cached_dir.children_len > 0) { + free(dh); + return NULL; + } + } + memcpy(dh->cached_dir.children, dh->dir->children, (dh->cached_dir.children_len * sizeof(ramfs_entry_t*))); + return dh; } @@ -609,18 +636,55 @@ void ramfs_closedir(ramfs_dh_t *dh) { assert(dh != NULL); + // cleanup dh->cached_dir + // free(dh->cached_dir); // Same lifetime as 'dh' + free(dh->cached_dir.children); + free(dh); } +// Relies on two assumptions: (currently doesn't; see below) +// - New elements are added at the end. +// - Elements remain in order when deleted +// +// TODO: Use the two assumptions above to be more efficient when a file is deleted. +// (currently it just searches the whole thing for every subsequent readdir, leading to O(n^2) behaviour const ramfs_entry_t *ramfs_readdir(ramfs_dh_t *dh) { assert(dh != NULL); - if (dh->loc < dh->dir->children_len) { - return dh->dir->children[dh->loc++]; - } - - return NULL; + if (dh->cached_loc >= dh->cached_dir.children_len) { + return NULL; + } + if (dh->loc >= dh->dir->children_len) { + dh->go_slow = 0; +// return NULL; + } + + if (!dh->go_slow) { + if (dh->dir->children[dh->loc] == dh->cached_dir.children[dh->cached_loc]) { + dh->cached_loc++; + return dh->dir->children[dh->loc++]; + } else { + dh->go_slow = 1; + } + } + + // Someone's been deleting files while the directory was open, so we're out of sync! + // (Adding files happens at the end of the iterator, so is not a problem) + // We need to find the entry in the original list to ensure it still exists (otherwise NULL-dereference!) + // This will slow things down, but at least we give the correct results. + for (int i=0; i < dh->dir->children_len; i++) { + if (dh->dir->children[i] == dh->cached_dir.children[dh->cached_loc]) { + dh->loc++; // redundant + dh->cached_loc++; + return dh->dir->children[i]; + } + } + + // The (cached) entry was deleted, so continue with the next entry... + dh->cached_loc++; + return ramfs_readdir(dh); // tail-recursion } void ramfs_seekdir(ramfs_dh_t *dh, long loc) @@ -727,20 +791,22 @@ int ramfs_rmdir(ramfs_entry_t *entry) return 0; } -void ramfs_rmtree(ramfs_entry_t *entry) +int ramfs_rmtree(ramfs_entry_t *entry) { assert(entry != NULL); if (ramfs_is_file(entry)) { - ramfs_unlink(entry); - return; + return ramfs_unlink(entry); } ramfs_dir_t *dir = (ramfs_dir_t *) entry; for (int i = 0; i < dir->children_len; i++) { if (ramfs_is_dir(dir->children[i])) { - ramfs_rmtree(dir->children[i]); + int ret = ramfs_rmtree(dir->children[i]); + if (ret != 0) { + return ret; + } } else { free(((ramfs_file_t *) dir->children[i])->data); free((void *) dir->children[i]->name); @@ -751,4 +817,5 @@ void ramfs_rmtree(ramfs_entry_t *entry) free((void *) entry->name); free(entry); } + return 0; } diff --git a/src/vfs.c b/src/vfs.c index c824d38..ebd83b8 100644 --- a/src/vfs.c +++ b/src/vfs.c @@ -20,10 +20,6 @@ static const char* TAG = "ramfs"; -#undef ESP_LOGV -#define ESP_LOGV ESP_LOGD - - #ifndef CONFIG_RAMFS_MAX_PARTITIONS # define CONFIG_RAMFS_MAX_PARTITIONS 1 #endif @@ -45,6 +41,12 @@ typedef struct { static ramfs_vfs_t *s_ramfs_vfs[CONFIG_RAMFS_MAX_PARTITIONS]; +// Cleanup functions (saves using goto) +static void cleanup_malloc_char(char** p) { + free(*p); +} + + static esp_err_t ramfs_get_empty(int *index) { int i; @@ -108,7 +110,7 @@ static ssize_t ramfs_vfs_read(void *ctx, int fd, void *data, size_t size) static int ramfs_vfs_open(void *ctx, const char *path, int flags, int mode) { - ESP_LOGV(TAG, "%s: path=\"%s\", flags=%x, mode=%x", __func__, path, flags, mode); + ESP_LOGV(TAG, "%s: path='%s', flags=%x, mode=%x", __func__, path, flags, mode); ramfs_vfs_t *vfs = (ramfs_vfs_t *) ctx; @@ -119,23 +121,25 @@ static int ramfs_vfs_open(void *ctx, const char *path, int flags, int mode) } } if (fd >= vfs->fh_len) { - ESP_LOGE(TAG, "open: no free file descriptors"); + ESP_LOGE(TAG, "%s: no free file descriptors", __func__); errno = ENFILE; return -1; } + int cache_errno = errno; ramfs_entry_t *entry = ramfs_get_entry(vfs->fs, path); + errno = cache_errno; if (NULL == entry) { if (flags & (O_CREAT | O_TRUNC)) { - ESP_LOGD(TAG, "%s: Create new file: path=\"%s\", flags=%x, mode=%x", __func__, path, flags, mode); + ESP_LOGD(TAG, "%s: Create new file: path='%s%s', flags=%x, mode=%x", __func__, vfs->base_path, path, flags, mode); entry = ramfs_create(vfs->fs, path, flags); if (NULL == entry) { - ESP_LOGW(TAG, "Failed to create new file at '%s': %s", path, strerror(errno)); + ESP_LOGW(TAG, "Failed to create new file at '%s%s': %s", vfs->base_path, path, strerror(errno)); return -1; } } else { - ESP_LOGW(TAG, "No file at '%s': %s", path, strerror(errno)); + ESP_LOGW(TAG, "No file at '%s%s': %s", vfs->base_path, path, strerror(errno)); return -1; } } @@ -185,11 +189,33 @@ static int ramfs_vfs_fstat(void *ctx, int fd, struct stat *st) #if defined(CONFIG_RAMFS_VFS_SUPPORT_DIR) static int ramfs_vfs_stat(void *ctx, const char *path, struct stat *st) { + ESP_LOGV(TAG, "%s: path='%s'", __func__, path); + ramfs_vfs_t *vfs = (ramfs_vfs_t *) ctx; ramfs_stat_t rst; - const ramfs_entry_t *entry = ramfs_get_entry(vfs->fs, path); + // Remove trailing slash if necessary + // TODO: Move this into ramfs proper + __attribute__((cleanup(cleanup_malloc_char))) + char* temp = NULL; + if (path[strlen(path)-1] == '/') { + temp = strdup(path); + if (NULL == temp) { + ESP_LOGV(TAG, "%s: RETURN=%d, errno=%d", __func__, -1, errno); + return -1; + } + temp[strlen(path)-1] = '\0'; + } + const char* norm_path = temp ? temp : path; // Normalised path + + // If the trailing slash was removed, then it must be a directory; error otherwise + bool trailing_slash_was_removed = (path[strlen(path)-1] == '/'); + + + ESP_LOGV(TAG, "calling %s(vfs->fs=%p, norm_path='%s'", "ramfs_get_entry", vfs->fs, norm_path); + const ramfs_entry_t *entry = ramfs_get_entry(vfs->fs, norm_path); if (entry == NULL) { + ESP_LOGV(TAG, "%s: RETURN=%d, errno=%d", __func__, -1, errno); return -1; } @@ -202,11 +228,23 @@ static int ramfs_vfs_stat(void *ctx, const char *path, struct stat *st) } else if (rst.type == RAMFS_ENTRY_TYPE_FILE) { st->st_mode |= S_IFREG; } + + // Trailing slash not allowed for files + if (trailing_slash_was_removed) { + if (false == S_ISDIR(st->st_mode)) { + errno = ENOTDIR; + return -1; + } + } + + ESP_LOGV(TAG, "%s: RETURN=%d", __func__, 0); return 0; } static int ramfs_vfs_unlink(void *ctx, const char *path) { + ESP_LOGV(TAG, "%s: path='%s'", __func__, path); + ramfs_vfs_t *vfs = (ramfs_vfs_t *) ctx; ramfs_entry_t *entry = ramfs_get_entry(vfs->fs, path); @@ -219,6 +257,8 @@ static int ramfs_vfs_unlink(void *ctx, const char *path) static int ramfs_vfs_rename(void *ctx, const char *src, const char *dst) { + ESP_LOGV(TAG, "%s: src='%s' -> dst='%s'", __func__, src, dst); + ramfs_vfs_t *vfs = (ramfs_vfs_t *) ctx; return ramfs_rename(vfs->fs, src, dst); @@ -226,18 +266,35 @@ static int ramfs_vfs_rename(void *ctx, const char *src, const char *dst) static DIR *ramfs_vfs_opendir(void *ctx, const char *path) { + ESP_LOGV(TAG, "%s: path='%s'", __func__, path); + ramfs_vfs_t *vfs = (ramfs_vfs_t *) ctx; ramfs_vfs_dh_t *dh; - const ramfs_entry_t *entry = ramfs_get_entry(vfs->fs, path); - if (entry == NULL) { - return NULL; - } + // Remove trailing slash if necessary + // TODO: Move this into ramfs proper + __attribute__((cleanup(cleanup_malloc_char))) + char* temp = NULL; + if (path[strlen(path)-1] == '/') { + temp = strdup(path); + if (NULL == temp) { + return NULL; + } + temp[strlen(path)-1] = '\0'; + } + const char* norm_path = temp ? temp : path; // Normalised path + + const ramfs_entry_t *entry = ramfs_get_entry(vfs->fs, norm_path); + if (NULL == entry) { + return NULL; + } ramfs_dh_t* ramfs_dh = ramfs_opendir(vfs->fs, entry); - if (!ramfs_dh) { - return NULL; - } + ESP_RETURN_ON_FALSE( + ramfs_dh != NULL, + NULL, + TAG, "failed to open directory entry=%p, path='%s': %s", entry, norm_path, strerror(errno) + ); dh = calloc(1, sizeof(*dh)); if (!dh) { @@ -247,6 +304,7 @@ static DIR *ramfs_vfs_opendir(void *ctx, const char *path) dh->dh = ramfs_dh; + ESP_LOGV(TAG, "%s: RETURN=%p", __func__, dh); return (DIR *) dh; } @@ -254,6 +312,8 @@ static int ramfs_vfs_readdir_r(void *ctx, DIR *pdir, struct dirent *entry, struct dirent **out_ent); static struct dirent *ramfs_vfs_readdir(void *ctx, DIR *pdir) { +// ESP_LOGV(TAG, "%s: pdir=%p", __func__, pdir); // Redundant, readdir_r prints the same info + ramfs_vfs_dh_t *dh = (ramfs_vfs_dh_t *) pdir; struct dirent *out_ent; @@ -268,16 +328,19 @@ static struct dirent *ramfs_vfs_readdir(void *ctx, DIR *pdir) static int ramfs_vfs_readdir_r(void *ctx, DIR *pdir, struct dirent *ent, struct dirent **out_ent) { + ESP_LOGV(TAG, "%s: pdir=%p, ent=%p", __func__, pdir, ent); + ramfs_vfs_dh_t *dh = (ramfs_vfs_dh_t *) pdir; const ramfs_entry_t *entry = ramfs_readdir(dh->dh); + ESP_LOGV(TAG, "%s: entry=%p", __func__, entry); if (entry == NULL) { *out_ent = NULL; return 0; } ent->d_ino = ramfs_telldir(dh->dh); - const char *name = ramfs_get_name(entry); + char *name = ramfs_get_name(entry); strlcpy(ent->d_name, name, sizeof(ent->d_name)); free(name); ent->d_type = DT_UNKNOWN; @@ -292,6 +355,8 @@ static int ramfs_vfs_readdir_r(void *ctx, DIR *pdir, struct dirent *ent, static long ramfs_vfs_telldir(void *ctx, DIR *pdir) { + ESP_LOGV(TAG, "%s: pdir=%p", __func__, pdir); + ramfs_vfs_dh_t *dh = (ramfs_vfs_dh_t *) pdir; return ramfs_telldir(dh->dh); @@ -299,6 +364,8 @@ static long ramfs_vfs_telldir(void *ctx, DIR *pdir) static void ramfs_vfs_seekdir(void *ctx, DIR *pdir, long offset) { + ESP_LOGV(TAG, "%s: pdir=%p, offset=%ld", __func__, pdir, offset); + ramfs_vfs_dh_t *dh = (ramfs_vfs_dh_t *) pdir; ramfs_seekdir(dh->dh, offset); @@ -307,30 +374,56 @@ static void ramfs_vfs_seekdir(void *ctx, DIR *pdir, long offset) __attribute__((nonnull)) static int ramfs_vfs_mkdir(void *ctx, const char *path, mode_t mode) { + ESP_LOGV(TAG, "%s: path='%s', mode=0x%x", __func__, path, (unsigned int)mode); + ramfs_vfs_t *vfs = (ramfs_vfs_t *) ctx; - if (NULL == ramfs_mkdir(vfs->fs, path)) { - ESP_LOGW(TAG, "cannot make directory '%s': %s", path, strerror(errno)); + // Remove trailing slash if necessary + // TODO: Move this into ramfs proper + __attribute__((cleanup(cleanup_malloc_char))) + char* temp = NULL; + if (path[strlen(path)-1] == '/') { + temp = strdup(path); + if (NULL == temp) { + return -1; + } + temp[strlen(path)-1] = '\0'; + } + const char* norm_path = temp ? temp : path; // Normalised path + + ESP_LOGV(TAG, "calling %s(vfs->fs=%p, norm_path='%s'", "ramfs_mkdir", vfs->fs, norm_path); + if (NULL == ramfs_mkdir(vfs->fs, norm_path)) { + ESP_LOGW(TAG, "cannot make directory '%s%s': %s", vfs->base_path, norm_path, strerror(errno)); return -1; - } + } + return 0; } __attribute__((nonnull)) static int ramfs_vfs_rmdir(void *ctx, const char *path) { + ESP_LOGV(TAG, "%s: path=%s", __func__, path); + ramfs_vfs_t *vfs = (ramfs_vfs_t *) ctx; ramfs_entry_t *entry = ramfs_get_entry(vfs->fs, path); if (entry == NULL) { + ESP_LOGV(TAG, "%s: failed (errno=%d)", __func__, errno); return -1; } - return ramfs_rmdir(entry); + int ret = ramfs_rmdir(entry); + if (ret != 0) { + ESP_LOGV(TAG, "%s: failed (errno=%d)", __func__, errno); + } + return ret; } static int ramfs_vfs_closedir(void *ctx, DIR *pdir) { + ESP_LOGV(TAG, "%s: pdir=%p", __func__, pdir); + ramfs_vfs_dh_t *dh = (ramfs_vfs_dh_t *) pdir; ramfs_closedir(dh->dh); diff --git a/test/test_ramfs.c b/test/test_ramfs.c index e82c377..d542272 100644 --- a/test/test_ramfs.c +++ b/test/test_ramfs.c @@ -48,7 +48,7 @@ static const char* SANITY = "sanity check for test-fixture"; //!< Marks t #define MOUNT_POINT "/ramfs" -ramfs_vfs_conf_t ramfs_vfs_conf = { +static ramfs_vfs_conf_t ramfs_vfs_conf = { .base_path = MOUNT_POINT, .fs = NULL, .max_files = 5, @@ -59,31 +59,47 @@ ramfs_vfs_conf_t ramfs_vfs_conf = { static bool file_exists(const char* path) { + int cache_errno = errno; + FILE* fd = fopen(path, "r"); if (NULL == fd) { + errno = cache_errno; return false; } fclose(fd); + + errno = cache_errno; return true; } static bool dir_exists(const char* path) { + int cache_errno = errno; + DIR* dir = opendir(path); if (dir == NULL) { + errno = cache_errno; return false; } closedir(dir); + + errno = cache_errno; return true; } - // ---- Cleanup functions ---- // // This will run before each test! static void LOCAL_TEST_setUp(void) { + // Failed tests may leave the filesystem initialised + if (ramfs_vfs_conf.fs != NULL) { + ESP_ERROR_CHECK(ramfs_vfs_unregister(&ramfs_vfs_conf)); + ramfs_deinit(ramfs_vfs_conf.fs); + ramfs_vfs_conf.fs = NULL; + } + // Create and mount ramfs_vfs_conf.fs = ramfs_init(); assert(ramfs_vfs_conf.fs != NULL); @@ -106,7 +122,7 @@ static void LOCAL_TEST_tearDown(void) // There were quite a few bugs though, so much of the below is TDD and can be relied upon -TEST_CASE("Register and Unregister file system", "[esp_ramfs][TDD-dev]") +TEST_CASE("Register and Unregister file system", "[esp_ramfs]") { LOCAL_TEST_setUp(); @@ -115,7 +131,7 @@ TEST_CASE("Register and Unregister file system", "[esp_ramfs][TDD-dev]") LOCAL_TEST_tearDown(); } -TEST_CASE("Create a file in the root directory", "[esp_ramfs][TDD-dev]") +TEST_CASE("Create a file in the root directory", "[esp_ramfs]") { LOCAL_TEST_setUp(); @@ -123,6 +139,7 @@ TEST_CASE("Create a file in the root directory", "[esp_ramfs][TDD-dev]") // Shouldn't be there yet TEST_ASSERT_FALSE(file_exists(filepath)); + errno = 0; // Open for writing, creates the file { @@ -139,9 +156,7 @@ TEST_CASE("Create a file in the root directory", "[esp_ramfs][TDD-dev]") } -// ---- Tests below never failed, and so are marked with the [not-TDD] tag - -TEST_CASE("Create a directory", "[esp_ramfs][not-TDD]") +TEST_CASE("Create a directory", "[esp_ramfs]") { LOCAL_TEST_setUp(); @@ -152,7 +167,9 @@ TEST_CASE("Create a directory", "[esp_ramfs][not-TDD]") // Create the directory { - mkdir(dirpath, 0755); + int ret = mkdir(dirpath, 0755); + TEST_ASSERT_EQUAL_MESSAGE(0, ret, SANITY); + TEST_ASSERT_EQUAL_MESSAGE(0, errno, SANITY); } TEST_ASSERT_TRUE(dir_exists(dirpath)); @@ -160,24 +177,424 @@ TEST_CASE("Create a directory", "[esp_ramfs][not-TDD]") LOCAL_TEST_tearDown(); } -TEST_CASE("Create a file in a directory", "[esp_ramfs][not-TDD]") +TEST_CASE("opendir() returns non-NULL on empty directories", "[esp_ramfs]") { LOCAL_TEST_setUp(); const char* dirpath = MOUNT_POINT "/dir"; - // Shouldn't be there yet - TEST_ASSERT_FALSE(dir_exists(dirpath)); + // Create the directory + { + int ret = mkdir(dirpath, 0755); + TEST_ASSERT_EQUAL_MESSAGE(0, ret, SANITY); + TEST_ASSERT_EQUAL_MESSAGE(0, errno, SANITY); + } + + { + DIR* dir = opendir(dirpath); + TEST_ASSERT_NOT_NULL_MESSAGE(dir, strerror(errno)); + TEST_ASSERT_EQUAL(0, errno); + + int ret = closedir(dir); + TEST_ASSERT_EQUAL_MESSAGE(0, ret, strerror(errno)); + TEST_ASSERT_EQUAL(0, errno); + } + + LOCAL_TEST_tearDown(); +} + +// Note: According to POSIX, a path ending in '/' will be interpreted as if it has a trailing '.' +// i.e. '/dir/subdir/' -> '/dir/subdit/.' This is significant when '/dir/subdir' is a symbolic link +// esp-idf does not support symbolic links, so we can simply reject any calls where a trailing is slash +// is given but it's not a directory. +TEST_CASE("stat() accepts trailing slash for directories", "[esp_ramfs]") +{ + LOCAL_TEST_setUp(); + + const char* dirpath = MOUNT_POINT "/dir"; + const char* dirpath_slash = MOUNT_POINT "/dir/"; // Create the directory { - mkdir(dirpath, 0755); + int ret = mkdir(dirpath, 0755); + TEST_ASSERT_EQUAL_MESSAGE(0, ret, SANITY); + TEST_ASSERT_EQUAL_MESSAGE(0, errno, SANITY); + } + + // Check it works without the slash + { + struct stat st; + int ret = stat(dirpath, &st); + TEST_ASSERT_EQUAL_MESSAGE(0, ret, SANITY); + TEST_ASSERT_EQUAL_MESSAGE(0, errno, SANITY); + } + + { + struct stat st; + int ret = stat(dirpath_slash, &st); + TEST_ASSERT_EQUAL(0, ret); + TEST_ASSERT_EQUAL(0, errno); } - TEST_ASSERT_TRUE(dir_exists(dirpath)); + LOCAL_TEST_tearDown(); +} + +// Note: According to POSIX, a path ending in '/' will be interpreted as if it has a trailing '.' +// i.e. '/dir/subdir/' -> '/dir/subdit/.' This is significant when '/dir/subdir' is a symbolic link +// esp-idf does not support symbolic links, so we can simply reject any calls where a trailing is slash +// is given but it's not a directory. +TEST_CASE("stat() should not accept trailing slash for files", "[esp_ramfs]") +{ + LOCAL_TEST_setUp(); + + const char* dirpath = MOUNT_POINT "/dir"; + const char* filepath = MOUNT_POINT "/dir" "/file"; + const char* filepath_slash = MOUNT_POINT "/dir" "/file/"; + + // Create directory and file + { + int ret = mkdir(dirpath, 0755); + TEST_ASSERT_EQUAL_MESSAGE(0, ret, SANITY); + TEST_ASSERT_EQUAL_MESSAGE(0, errno, SANITY); + + FILE* fd = fopen(filepath, "a"); + TEST_ASSERT_NOT_NULL_MESSAGE(fd, SANITY); + ret = fclose(fd); + TEST_ASSERT_EQUAL_MESSAGE(0, ret, SANITY); + } + + // Check it works without the slash + { + struct stat st; + int ret = stat(dirpath, &st); + TEST_ASSERT_EQUAL_MESSAGE(0, ret, SANITY); + TEST_ASSERT_EQUAL_MESSAGE(0, errno, SANITY); + } + + { + struct stat st; + int ret = stat(filepath_slash, &st); + TEST_ASSERT_EQUAL(-1, ret); + TEST_ASSERT_EQUAL(ENOTDIR, errno); + } + + LOCAL_TEST_tearDown(); +} + +// Note: According to POSIX, a path ending in '/' will be interpreted as if it has a trailing '.' +// i.e. '/dir/subdir/' -> '/dir/subdit/.' This is significant when '/dir/subdir' is a symbolic link +// esp-idf does not support symbolic links, so we can simply reject any calls where a trailing is slash +// is given but it's not a directory. +TEST_CASE("mkdir(), opendir(), and rmdir() accepts trailing slash", "[esp_ramfs]") +{ + LOCAL_TEST_setUp(); + + const char* dirpath = MOUNT_POINT "/dir/"; + + // Create + { + int ret = mkdir(dirpath, 0755); + TEST_ASSERT_EQUAL_MESSAGE(0, ret, SANITY); // Never failed + TEST_ASSERT_EQUAL_MESSAGE(0, errno, SANITY); // Never failed + } + + // Open + { + DIR* dir = opendir(dirpath); + TEST_ASSERT_NOT_NULL_MESSAGE(dir, strerror(errno)); + TEST_ASSERT_EQUAL(0, errno); + + int ret = closedir(dir); + TEST_ASSERT_EQUAL_MESSAGE(0, ret, strerror(errno)); + TEST_ASSERT_EQUAL(0, errno); + } + + // Remove + { + int ret = rmdir(dirpath); + TEST_ASSERT_EQUAL_MESSAGE(0, ret, SANITY); // Never failed + TEST_ASSERT_EQUAL_MESSAGE(0, errno, SANITY); // Never failed + } + + // Check it is gone + TEST_ASSERT_FALSE_MESSAGE(dir_exists(dirpath), SANITY); // Never failed + + LOCAL_TEST_tearDown(); +} + +TEST_CASE("Create a file in a directory", "[esp_ramfs]") +{ + LOCAL_TEST_setUp(); + + const char* dirpath = MOUNT_POINT "/dir"; + const char* filepath = MOUNT_POINT "/dir" "/file.txt"; + + + mkdir(dirpath, 0755); + TEST_ASSERT_TRUE_MESSAGE(dir_exists(dirpath), SANITY); + + FILE* fd = fopen(filepath, "w"); + TEST_ASSERT_NOT_NULL_MESSAGE(fd, SANITY); // Never failed + TEST_ASSERT_EQUAL(0, errno); + int ret = fclose(fd); + TEST_ASSERT_EQUAL_MESSAGE(0, ret, SANITY); // Never failed + TEST_ASSERT_EQUAL_MESSAGE(0, errno, SANITY); // Never failed + + TEST_ASSERT_TRUE_MESSAGE(file_exists(filepath), SANITY); // Never failed LOCAL_TEST_tearDown(); } +TEST_CASE("Iterate through a directory", "[esp_ramfs]") +{ + LOCAL_TEST_setUp(); + + const char* dirpath = MOUNT_POINT "/dir/"; + mkdir(dirpath, 0755); + + const int number_of_files = 3; + const char* file_path[3] = { + MOUNT_POINT "/dir/" "file1.txt", + MOUNT_POINT "/dir/" "submarine.txt", + MOUNT_POINT "/dir/" "file3.sub", + }; + for (int i = 0; i < number_of_files; i++) { + FILE* fd = fopen(file_path[i], "w"); + TEST_ASSERT_NOT_NULL(fd); + + int ret = fclose(fd); + TEST_ASSERT_EQUAL(0, ret); + } + + int count = 0; + + DIR* dir = opendir(dirpath); // uses approximately 596 bytes of heap memory (vfs_fat_dir_t) + TEST_ASSERT_NOT_NULL_MESSAGE(dir, SANITY); + struct dirent * entry; + while ((entry = readdir(dir)) != NULL) { + ESP_LOGI(TAG, "Found entry: '%s'", entry->d_name); + count++; + } + TEST_ASSERT_EQUAL_MESSAGE(number_of_files, count, SANITY); // Never failed + + + + LOCAL_TEST_tearDown(); +} + +// POSIX specifications for readdir: +// +// "If a file is removed from or added to the directory after the most recent call +// to opendir() or rewinddir(), whether a subsequent call to readdir() returns an +// entry for that file is unspecified." +// +// Note "that file", meaning that other files are unaffected. + +TEST_CASE("Delete files as we iterate a directory", "[esp_ramfs]") +{ + LOCAL_TEST_setUp(); + + const char* dirpath = MOUNT_POINT "/dir/"; + mkdir(dirpath, 0755); + + // With enough of them, the pattern of failures becomes more obvious + const int number_of_files = 10; + const char* file_path[10] = { + MOUNT_POINT "/dir/" "0", + MOUNT_POINT "/dir/" "1", + MOUNT_POINT "/dir/" "2", + MOUNT_POINT "/dir/" "3", + MOUNT_POINT "/dir/" "4", + MOUNT_POINT "/dir/" "5", + MOUNT_POINT "/dir/" "6", + MOUNT_POINT "/dir/" "7", + MOUNT_POINT "/dir/" "8", + MOUNT_POINT "/dir/" "9", + }; + for (int i = 0; i < number_of_files; i++) { + FILE* fd = fopen(file_path[i], "w"); + TEST_ASSERT_NOT_NULL(fd); + + int ret = fclose(fd); + TEST_ASSERT_EQUAL(0, ret); + } + + char scratch_path[100]; + strncpy(scratch_path, dirpath, 100); + + + DIR* dir = opendir(dirpath); // uses approximately 596 bytes of heap memory (vfs_fat_dir_t) + TEST_ASSERT_NOT_NULL_MESSAGE(dir, SANITY); + + struct dirent * entry; + int count = 0; + while ((entry = readdir(dir)) != NULL) { + ESP_LOGI(TAG, "found entry: '%s'", entry->d_name); + scratch_path[strlen(dirpath)] = '\0'; // No need to write the directory each time + strlcat(scratch_path, entry->d_name, 100); + +// TEST_ASSERT_EQUAL_STRING(file_path[count], scratch_path); // Check it's the right one + + ESP_LOGI(TAG, "Deleting: '%s'", scratch_path); + + + int ret = unlink(scratch_path); + TEST_ASSERT_EQUAL_MESSAGE(0, ret, scratch_path); + count++; + } + int ret = closedir(dir); + TEST_ASSERT_EQUAL_MESSAGE(0, ret, strerror(errno)); + + TEST_ASSERT_EQUAL(number_of_files, count); + + + + LOCAL_TEST_tearDown(); +} + +TEST_CASE("Delete multiple files behind the iterator", "[esp_ramfs]") +{ + LOCAL_TEST_setUp(); + + int ret; + + const char* dirpath = MOUNT_POINT "/dir/"; + mkdir(dirpath, 0755); + + // With enough of them, the pattern of failures becomes more obvious + const int number_of_files = 10; + const char* file_path[10] = { + MOUNT_POINT "/dir/" "0", + MOUNT_POINT "/dir/" "1", + MOUNT_POINT "/dir/" "2", + MOUNT_POINT "/dir/" "3", + MOUNT_POINT "/dir/" "4", + MOUNT_POINT "/dir/" "5", + MOUNT_POINT "/dir/" "6", + MOUNT_POINT "/dir/" "7", + MOUNT_POINT "/dir/" "8", + MOUNT_POINT "/dir/" "9", + }; + for (int i = 0; i < number_of_files; i++) { + FILE* fd = fopen(file_path[i], "w"); + TEST_ASSERT_NOT_NULL(fd); + + ret = fclose(fd); + TEST_ASSERT_EQUAL(0, ret); + } + + char scratch_path[100]; + strncpy(scratch_path, dirpath, 100); + + DIR* dir = opendir(dirpath); // uses approximately 596 bytes of heap memory (vfs_fat_dir_t) + TEST_ASSERT_NOT_NULL_MESSAGE(dir, SANITY); + + struct dirent * entry; + int count = 0; + // Read several entries + for (int i=0; i < 3; i++) { + entry = readdir(dir); + count++; + } + // Delete two of the already read ones + ret = unlink(file_path[0]); + ret = unlink(file_path[1]); + + + while ((entry = readdir(dir)) != NULL) { + ESP_LOGI(TAG, "found entry: '%s'", entry->d_name); + scratch_path[strlen(dirpath)] = '\0'; // No need to write the directory each time + strlcat(scratch_path, entry->d_name, 100); + + TEST_ASSERT_EQUAL_STRING(file_path[count], scratch_path); // Check it's the right one + + count++; + } + ret = closedir(dir); + TEST_ASSERT_EQUAL_MESSAGE(0, ret, strerror(errno)); + + TEST_ASSERT_EQUAL(number_of_files, count); + + + + LOCAL_TEST_tearDown(); +} + +// A test case for deleting in front of the iterator is not included, because it is 'unspecified behaviour' +// i.e. the deleted files can be listed anyway, not listed, or a mixture of both! + +TEST_CASE("Delete behind and in front of the iterator", "[esp_ramfs]") +{ + LOCAL_TEST_setUp(); + + int ret; + + const char* dirpath = MOUNT_POINT "/dir/"; + mkdir(dirpath, 0755); + + // With enough of them, the pattern of failures becomes more obvious + const int number_of_files = 10; + const char* file_path[10] = { + MOUNT_POINT "/dir/" "0", + MOUNT_POINT "/dir/" "1", + MOUNT_POINT "/dir/" "2", + MOUNT_POINT "/dir/" "3", + MOUNT_POINT "/dir/" "4", + MOUNT_POINT "/dir/" "5", + MOUNT_POINT "/dir/" "6", + MOUNT_POINT "/dir/" "7", + MOUNT_POINT "/dir/" "8", + MOUNT_POINT "/dir/" "9", + }; + for (int i = 0; i < number_of_files; i++) { + FILE* fd = fopen(file_path[i], "w"); + TEST_ASSERT_NOT_NULL(fd); + + ret = fclose(fd); + TEST_ASSERT_EQUAL(0, ret); + } + + char scratch_path[100]; + strncpy(scratch_path, dirpath, 100); + + DIR* dir = opendir(dirpath); // uses approximately 596 bytes of heap memory (vfs_fat_dir_t) + TEST_ASSERT_NOT_NULL_MESSAGE(dir, SANITY); + + struct dirent * entry; + int count = 0; + // Read several entries + for (int i=0; i < 3; i++) { + entry = readdir(dir); + count++; + } + // Delete two of the already read ones + ret = unlink(file_path[1]); + ret = unlink(file_path[6]); + + + while ((entry = readdir(dir)) != NULL) { + ESP_LOGI(TAG, "entry[%d]: '%s'", count, entry->d_name); + scratch_path[strlen(dirpath)] = '\0'; // No need to write the directory each time + strlcat(scratch_path, entry->d_name, 100); + + // Whether a file added/removed after the call to opendir() is returned by subsequent readdir is unspecified. + // The 'implementation' doesn't even need to Ddocument it! Or consistently do one, and not the other. + if (count == 6) { + count++; // In our case, though, it will consistently skip deleted directories. + } + + TEST_ASSERT_EQUAL_STRING(file_path[count], scratch_path); + + count++; + } + ret = closedir(dir); + TEST_ASSERT_EQUAL_MESSAGE(0, ret, strerror(errno)); + + TEST_ASSERT_EQUAL(number_of_files, count); + + + + LOCAL_TEST_tearDown(); +} From 78f13f5fa58d264f88b8677b6309c1d47a7d6f60 Mon Sep 17 00:00:00 2001 From: Vincent Dalstra Date: Tue, 3 Mar 2026 16:36:48 +0800 Subject: [PATCH 14/17] Increased the default max filesystems to 4 THis was done to make the Unit-test app simpler; to speed up tests, the the file system is generally left mounted after each test. But the ramfs unit tests use a different path, and so try to setup a second filesystem, exceeding the previous maximum of just 1. Furthermore, it's only 24 bytes of static memory per additional slot, so there's very little harm in increasing it a bit. --- Kconfig | 2 +- src/vfs.c | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/Kconfig b/Kconfig index 5c1153a..d0b7873 100644 --- a/Kconfig +++ b/Kconfig @@ -9,7 +9,7 @@ config RAMFS_USE_RBTREE config RAMFS_MAX_PARTITIONS int "Max partitions" - default 1 + default 4 help This option specifies the number of partitions that can be mounted using VFS at the same time. diff --git a/src/vfs.c b/src/vfs.c index ebd83b8..a2037fe 100644 --- a/src/vfs.c +++ b/src/vfs.c @@ -21,7 +21,7 @@ static const char* TAG = "ramfs"; #ifndef CONFIG_RAMFS_MAX_PARTITIONS -# define CONFIG_RAMFS_MAX_PARTITIONS 1 +# define CONFIG_RAMFS_MAX_PARTITIONS 4 #endif #if defined(CONFIG_RAMFS_VFS_SUPPORT_DIR) From c2cf1e4e315e41f0900dfde0e46901f28d1f9871 Mon Sep 17 00:00:00 2001 From: Vincent Dalstra Date: Mon, 16 Mar 2026 10:35:09 +0800 Subject: [PATCH 15/17] Doc; improve top-of-file message --- test/test_ramfs.c | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/test/test_ramfs.c b/test/test_ramfs.c index d542272..1431207 100644 --- a/test/test_ramfs.c +++ b/test/test_ramfs.c @@ -1,7 +1,15 @@ -/* Not developed with TDD! +/* test harness for ramfs. * - * Instead, these are just tests of an existing open-source library. Added after I found - * that the mkdir() function wasn't working! + * Mostly calls ramfs/vfs.h. Indirectly covers vfs.c, ramfs.c + * + * Note: not developed with TDD; rather, this was an existing library, but when using it I found + * that the mkdir() function wasn't working! So, this test harness was written to help me + * debug and fix it. That's why almost all the tests are about directories. + * + * Note: Assume every single TEST_ASSERT statements has a purpose. All tests* have, at some point, + * failed and required code to 'fix' (i.e. implement the correct functionality). Exception is tests + * marked SANITY - these are either copies of stuff already tested in a previous TEST_CASE, or they + * were added to aid in debugging. */ // ---- Test framework ---- // From d1741e4fc95d17ff6d43b19cdf3391ad196455c0 Mon Sep 17 00:00:00 2001 From: Vincent Dalstra Date: Mon, 16 Mar 2026 10:47:45 +0800 Subject: [PATCH 16/17] Doc: improve comments in unit_test CMakeLists.txt Remove lines with commented-out dependencies. Add a note about how dependencies are small on purpose. --- test/CMakeLists.txt | 9 +++------ test/test_ramfs.c | 3 ++- 2 files changed, 5 insertions(+), 7 deletions(-) diff --git a/test/CMakeLists.txt b/test/CMakeLists.txt index 98bdbd7..b3bc9db 100644 --- a/test/CMakeLists.txt +++ b/test/CMakeLists.txt @@ -9,15 +9,12 @@ idf_component_register( # Any other components needed by the tests go under here PRIV_REQUIRES -# code_snippets -# fs_utils - # Useful for measuring performance # esp_timer - # This component sets up eMMC memory/filesystem - PRIV_REQUIRES testfixture_emmc_fatfs - + # Dependencies have been deliberately kept low + # esp-idf dependencies (e.g. vfs) are ok, but there's no need to add them + # if they are not used directly. ) target_compile_options(${COMPONENT_LIB} PRIVATE -Werror) # Doesn't work? diff --git a/test/test_ramfs.c b/test/test_ramfs.c index 1431207..20bcd94 100644 --- a/test/test_ramfs.c +++ b/test/test_ramfs.c @@ -64,7 +64,7 @@ static ramfs_vfs_conf_t ramfs_vfs_conf = { // ---- Forward-declared static functions ---- // - +// Deliberately avoids using stat() static bool file_exists(const char* path) { int cache_errno = errno; @@ -80,6 +80,7 @@ static bool file_exists(const char* path) return true; } +// Deliberately avoids using stat() static bool dir_exists(const char* path) { int cache_errno = errno; From 17aa086284367024c54a28c37a05848b83ce788d Mon Sep 17 00:00:00 2001 From: Vincent Dalstra Date: Sat, 18 Apr 2026 23:49:52 +0800 Subject: [PATCH 17/17] Add existing tests, so they run under Unity Replace assert() statements with the corresponding TEST_ASSERT_NULL or TEST_ASSERT_NOT_NULL or TEST_ASSERT_EQUAL statements. --- test/preexisting_tests.c | 363 +++++++++++++++++++++++++++++++++++++++ 1 file changed, 363 insertions(+) create mode 100644 test/preexisting_tests.c diff --git a/test/preexisting_tests.c b/test/preexisting_tests.c new file mode 100644 index 0000000..e9ba907 --- /dev/null +++ b/test/preexisting_tests.c @@ -0,0 +1,363 @@ +/* + * + */ + +// ---- Test framework ---- // +#include "unity.h" + +// ---- Code to be tested ---- // +#include "ramfs/vfs.h" + +// ---- Additional test fixtures ---- // + +// ---- esp-idf components & system headers ---- // +#include +#include +#include +#include +#include + + +#include "esp_err.h" +#include "esp_check.h" +#include "esp_log.h" + +#include "esp_system.h" // Allows measuring currently available memory + +// ---- Local files (this component) ---- // + +// ---- End of includes ---- // + +// Tag prepended to ESP_LOG messages in this component: can also be used to filter messages at runtime +__attribute__((unused)) +static const char* TAG = "test_ramfs"; + +__attribute__((unused)) +static const char* SANITY = "sanity check for test-fixture"; //!< Marks tests which are to ensure the test-fixture is working correctly (not TDD) + +// ---- private types ---- // + +// ---- private Global variables ---- // + + +// ---- Forward-declared static functions ---- // + +// ---- Cleanup functions ---- // + +// This will run before each test! +static void LOCAL_TEST_setUp(void) +{ + errno = 0; +} + +// This will run after each test! +static void LOCAL_TEST_tearDown(void) +{ + ; +} + + +// ---- Tests ---- // + + + +TEST_CASE("init", "[esp_ramfs]") +{ + LOCAL_TEST_setUp(); + + ramfs_fs_t *fs; + + fs = ramfs_init(); + TEST_ASSERT_NOT_NULL(fs); + + LOCAL_TEST_tearDown(); +} + +TEST_CASE("deinit", "[esp_ramfs]") +{ + LOCAL_TEST_setUp(); + + ramfs_fs_t *fs; + + fs = ramfs_init(); + TEST_ASSERT_NOT_NULL(fs); + + ramfs_deinit(fs); + fs = NULL; + + LOCAL_TEST_tearDown(); +} + +TEST_CASE("create", "[esp_ramfs]") +{ + LOCAL_TEST_setUp(); + + ramfs_fs_t *fs; + ramfs_entry_t *file; + + fs = ramfs_init(); + TEST_ASSERT_NOT_NULL(fs); + + file = ramfs_create(fs, "test", 0); + TEST_ASSERT_NOT_NULL(file); + + ramfs_deinit(fs); + fs = NULL; + + LOCAL_TEST_tearDown(); +} + + + +TEST_CASE("unlink", "[esp_ramfs]") +{ + LOCAL_TEST_setUp(); + + ramfs_fs_t *fs; + ramfs_entry_t *file; + + fs = ramfs_init(); + TEST_ASSERT_NOT_NULL(fs); + + file = ramfs_create(fs, "test", 0); + TEST_ASSERT_NOT_NULL(file); + + TEST_ASSERT_EQUAL(0, ramfs_unlink(file)); + + ramfs_deinit(fs); + fs = NULL; + + LOCAL_TEST_tearDown(); +} + +TEST_CASE("open", "[esp_ramfs]") +{ + LOCAL_TEST_setUp(); + + ramfs_fs_t *fs; + ramfs_entry_t *file; + ramfs_fh_t *fh; + + fs = ramfs_init(); + TEST_ASSERT_NOT_NULL(fs); + + file = ramfs_create(fs, "test", 0); + TEST_ASSERT_NOT_NULL(file); + + fh = ramfs_open(fs, file, 0); + TEST_ASSERT_NOT_NULL(fh); + + ramfs_close(fh); + + ramfs_deinit(fs); + fs = NULL; + + LOCAL_TEST_tearDown(); +} + +TEST_CASE("read", "[esp_ramfs]") +{ + LOCAL_TEST_setUp(); + + ramfs_fs_t *fs; + ramfs_entry_t *file; + ramfs_fh_t *fh; + char buf[12]; + + fs = ramfs_init(); + TEST_ASSERT_NOT_NULL(fs); + + file = ramfs_create(fs, "test", 0); + TEST_ASSERT_NOT_NULL(file); + + fh = ramfs_open(fs, file, O_WRONLY); + TEST_ASSERT_NOT_NULL(fh); + + TEST_ASSERT_EQUAL(12, ramfs_write(fh, "Hello World!", 12)); + + ramfs_close(fh); + + fh = ramfs_open(fs, file, O_RDONLY); + TEST_ASSERT_NOT_NULL(fh); + + TEST_ASSERT_EQUAL(12, ramfs_read(fh, buf, 12)); + + TEST_ASSERT_EQUAL(0, memcmp(buf, "Hello World!", 12)); + + ramfs_close(fh); + + ramfs_deinit(fs); + fs = NULL; + + LOCAL_TEST_tearDown(); +} + +TEST_CASE("write", "[esp_ramfs]") +{ + LOCAL_TEST_setUp(); + + ramfs_fs_t *fs; + ramfs_entry_t *file; + ramfs_fh_t *fh; + char buf[12]; + + fs = ramfs_init(); + TEST_ASSERT_NOT_NULL(fs); + + file = ramfs_create(fs, "test", 0); + TEST_ASSERT_NOT_NULL(file); + + fh = ramfs_open(fs, file, O_WRONLY); + TEST_ASSERT_NOT_NULL(fh); + + TEST_ASSERT_EQUAL(12, ramfs_write(fh, "Hello World!", 12)); + + ramfs_close(fh); + + fh = ramfs_open(fs, file, O_RDONLY); + TEST_ASSERT_NOT_NULL(fh); + + TEST_ASSERT_EQUAL(12, ramfs_read(fh, buf, 12)); + + TEST_ASSERT_EQUAL(0, memcmp(buf, "Hello World!", 12)); + + ramfs_close(fh); + + ramfs_deinit(fs); + fs = NULL; + + LOCAL_TEST_tearDown(); +} + +TEST_CASE("seek", "[esp_ramfs]") +{ + LOCAL_TEST_setUp(); + + ramfs_fs_t *fs; + ramfs_entry_t *file; + ramfs_fh_t *fh; + char buf[12]; + + fs = ramfs_init(); + TEST_ASSERT_NOT_NULL(fs); + + file = ramfs_create(fs, "test", 0); + TEST_ASSERT_NOT_NULL(file); + + fh = ramfs_open(fs, file, O_RDWR); + TEST_ASSERT_NOT_NULL(fh); + + TEST_ASSERT_EQUAL(12, ramfs_write(fh, "Hello World!", 12)); + + TEST_ASSERT_EQUAL(6, ramfs_seek(fh, -6, SEEK_CUR)); + + TEST_ASSERT_EQUAL(5, ramfs_write(fh, "There", 5)); + + TEST_ASSERT_EQUAL(0, ramfs_seek(fh, 0, SEEK_SET)); + + TEST_ASSERT_EQUAL(12, ramfs_read(fh, buf, 12)); + + TEST_ASSERT_EQUAL(0, memcmp("Hello There!", buf, 12)); + + ramfs_close(fh); + + ramfs_deinit(fs); + fs = NULL; + + LOCAL_TEST_tearDown(); +} + +TEST_CASE("mkdir", "[esp_ramfs]") +{ + LOCAL_TEST_setUp(); + + ramfs_fs_t *fs; + ramfs_entry_t *dir; + + fs = ramfs_init(); + TEST_ASSERT_NOT_NULL(fs); + + dir = ramfs_mkdir(fs, "test"); + TEST_ASSERT_NOT_NULL(dir); + + ramfs_deinit(fs); + fs = NULL; + + LOCAL_TEST_tearDown(); +} + +TEST_CASE("rmdir", "[esp_ramfs]") +{ + LOCAL_TEST_setUp(); + + ramfs_fs_t *fs; + ramfs_entry_t *dir; + + fs = ramfs_init(); + TEST_ASSERT_NOT_NULL(fs); + + dir = ramfs_mkdir(fs, "test"); + TEST_ASSERT_NOT_NULL(dir); + + TEST_ASSERT_EQUAL(0, ramfs_rmdir(dir)); + + ramfs_deinit(fs); + fs = NULL; + + LOCAL_TEST_tearDown(); +} + +TEST_CASE("issue 1", "[esp_ramfs]") +{ + LOCAL_TEST_setUp(); + + ramfs_fs_t *fs; + ramfs_entry_t *dir, *file; + ramfs_fh_t *fh; + char buf[25]; + + fs = ramfs_init(); + TEST_ASSERT_NOT_NULL(fs); + + dir = ramfs_mkdir(fs, "dir"); + TEST_ASSERT_NOT_NULL(dir); + + file = ramfs_create(fs, "dir/test_file.txt", 0); + TEST_ASSERT_NOT_NULL(file); + + fh = ramfs_open(fs, file, O_WRONLY); + TEST_ASSERT_NOT_NULL(fh); + + TEST_ASSERT_EQUAL(25, ramfs_write(fh, "This is dir/test_file.txt", 25)); + + ramfs_close(fh); + + TEST_ASSERT_EQUAL(0, ramfs_rename(fs, "dir/test_file.txt", "dir/test_file_new.txt")); + + file = ramfs_get_entry(fs, "dir/test_file_new.txt"); + TEST_ASSERT_NOT_NULL(file); + + fh = ramfs_open(fs, file, O_RDONLY); + TEST_ASSERT_NOT_NULL(fh); + + TEST_ASSERT_EQUAL(25, ramfs_read(fh, buf, 25)); + + TEST_ASSERT_EQUAL(0, memcmp(buf, "This is dir/test_file.txt", 25)); + + ramfs_close(fh); + + file = ramfs_get_entry(fs, "dir/test_file.txt"); + TEST_ASSERT_NULL(file); + + file = ramfs_get_entry(fs, "dir/test_file_new.txt"); + TEST_ASSERT_NOT_NULL(file); + TEST_ASSERT_EQUAL(0, ramfs_unlink(file)); + + file = ramfs_get_entry(fs, "dir/test_file_new.txt"); + TEST_ASSERT_NULL(file); + + ramfs_deinit(fs); + fs = NULL; + + LOCAL_TEST_tearDown(); +}