From 0e9a5237bdd1009572cf3e280fb07ddec7c4a02a Mon Sep 17 00:00:00 2001 From: Jamie Counsell Date: Sun, 25 Oct 2026 11:11:34 -0400 Subject: [PATCH 1/2] Fix inverted rmdir() result in the POSIX and stdio adapters rmdir() in the POSIX and stdio adapters returned false when the directory was removed and true when it wasn't, the opposite of every other adapter (and of mkdir()). Adds test/test_posix_fs, which runs the adapters against a real temp directory. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_019rUgoKJdB4U8fNS9HybY8S --- include/microStore/Adapters/PosixFileSystem.h | 5 +- include/microStore/Adapters/StdioFileSystem.h | 5 +- test/test_posix_fs/test_posix_fs.cpp | 75 +++++++++++++++++++ 3 files changed, 77 insertions(+), 8 deletions(-) create mode 100644 test/test_posix_fs/test_posix_fs.cpp diff --git a/include/microStore/Adapters/PosixFileSystem.h b/include/microStore/Adapters/PosixFileSystem.h index 9c58927..ee9335f 100644 --- a/include/microStore/Adapters/PosixFileSystem.h +++ b/include/microStore/Adapters/PosixFileSystem.h @@ -315,10 +315,7 @@ class PosixFileSystem : public microStore::FileSystem { } inline virtual bool rmdir(const char* path) override { - if (::rmdir(path) == 0) { - return false; - } - return true; + return (::rmdir(path) == 0); } diff --git a/include/microStore/Adapters/StdioFileSystem.h b/include/microStore/Adapters/StdioFileSystem.h index cecffb0..9e54e7d 100644 --- a/include/microStore/Adapters/StdioFileSystem.h +++ b/include/microStore/Adapters/StdioFileSystem.h @@ -303,10 +303,7 @@ class StdioFileSystem : public microStore::FileSystem { } inline virtual bool rmdir(const char* path) override { - if (::rmdir(path) == 0) { - return false; - } - return true; + return (::rmdir(path) == 0); } diff --git a/test/test_posix_fs/test_posix_fs.cpp b/test/test_posix_fs/test_posix_fs.cpp new file mode 100644 index 0000000..68d2bff --- /dev/null +++ b/test/test_posix_fs/test_posix_fs.cpp @@ -0,0 +1,75 @@ +/* + * Tests for the POSIX and stdio filesystem adapters against a real + * temporary directory. + */ + +#include +#include +#include + +#include +#include +#include +#include + +static std::string tmp; + +static bool is_dir(const std::string& path) { + struct stat st; + return ::stat(path.c_str(), &st) == 0 && S_ISDIR(st.st_mode); +} + +/* ---- rmdir ---- */ + +static void check_rmdir(microStore::FileSystem fs) { + const std::string dir = tmp + "/sub"; + TEST_ASSERT_TRUE(fs.mkdir(dir.c_str())); + TEST_ASSERT_TRUE(fs.rmdir(dir.c_str())); + TEST_ASSERT_FALSE(is_dir(dir)); + TEST_ASSERT_FALSE(fs.rmdir(dir.c_str())); +} + +void test_posix_rmdir_reports_result() { + check_rmdir(microStore::Adapters::PosixFileSystem()); +} + +void test_stdio_rmdir_reports_result() { + check_rmdir(microStore::Adapters::StdioFileSystem()); +} + +/* ---- Main ---- */ + +void setUp() { + char tmpl[] = "/tmp/microstore_fs_XXXXXX"; + TEST_ASSERT_NOT_NULL(::mkdtemp(tmpl)); + tmp = tmpl; +} + +void tearDown() { + std::string cmd = "rm -rf '" + tmp + "'"; + (void)std::system(cmd.c_str()); +} + +int runUnityTests(void) { + UNITY_BEGIN(); + RUN_TEST(test_posix_rmdir_reports_result); + RUN_TEST(test_stdio_rmdir_reports_result); + return UNITY_END(); +} + +// For native dev-platform or for some embedded frameworks +int main(void) { + return runUnityTests(); +} + +#ifdef ARDUINO +// For Arduino framework +void setup() { + // Wait ~2 seconds before the Unity test runner + // establishes connection with a board Serial interface + delay(2000); + + runUnityTests(); +} +void loop() {} +#endif From 6f0c4e08e286bfafc44a03b9eb3ae085e85d6805 Mon Sep 17 00:00:00 2001 From: Jamie Counsell Date: Sun, 25 Oct 2026 11:13:44 -0400 Subject: [PATCH 2/2] Resolve POSIX adapter paths against the base path The POSIX adapter took a base path but ignored it (outside ESP32's LittleFS.begin()), so relative store paths like ./known_store/ resolved against the working directory. On ESP-IDF there is no working directory, so these paths never resolved. The adapter also kept the caller's pointer, which dangles if the string was only temporary. The adapter now keeps its own copy of the base path and resolves relative paths against it. Absolute paths, and everything when the base path is empty (the default), are unchanged. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_019rUgoKJdB4U8fNS9HybY8S --- include/microStore/Adapters/PosixFileSystem.h | 48 ++++++++++++---- test/test_posix_fs/test_posix_fs.cpp | 57 +++++++++++++++++++ 2 files changed, 93 insertions(+), 12 deletions(-) diff --git a/include/microStore/Adapters/PosixFileSystem.h b/include/microStore/Adapters/PosixFileSystem.h index ee9335f..78bcde2 100644 --- a/include/microStore/Adapters/PosixFileSystem.h +++ b/include/microStore/Adapters/PosixFileSystem.h @@ -32,6 +32,7 @@ #include #include #include +#include namespace microStore { namespace Adapters { @@ -198,7 +199,7 @@ class PosixFileSystem : public microStore::FileSystem { class FileSystemImpl : public microStore::FileSystemImpl { public: - FileSystemImpl(const char* basepath = "") : _basepath(basepath) {} + FileSystemImpl(const char* basepath = "") : _basepath(basepath ? basepath : "") {} virtual ~FileSystemImpl() {} public: @@ -220,7 +221,7 @@ class PosixFileSystem : public microStore::FileSystem { USTORE_LOG("[ustore] Initializing PosixFileSystem\n"); #if defined(ESP32) // Initialize LittleFS for POSIX file access - if (!LittleFS.begin(true, _basepath)) { + if (!LittleFS.begin(true, _basepath.c_str())) { USTORE_LOG("[ustore] Failed to initialize PosixFileSystem!\n"); return false; } @@ -247,7 +248,9 @@ class PosixFileSystem : public microStore::FileSystem { return true; } - virtual microStore::File open(const char* path, microStore::File::Mode mode, const bool create = false) override { + virtual microStore::File open(const char* path_in, microStore::File::Mode mode, const bool create = false) override { + const std::string full = resolve(path_in); + const char* path = full.c_str(); int flags; switch (mode) { // Read only. File must exist. ("r") @@ -283,7 +286,9 @@ class PosixFileSystem : public microStore::FileSystem { } - inline virtual bool exists(const char* path) override { + inline virtual bool exists(const char* path_in) override { + const std::string full = resolve(path_in); + const char* path = full.c_str(); int fd = ::open(path, O_RDONLY); if (fd != -1) { ::close(fd); @@ -292,15 +297,19 @@ class PosixFileSystem : public microStore::FileSystem { return false; } - inline virtual bool remove(const char* path) override { + inline virtual bool remove(const char* path_in) override { + const std::string full = resolve(path_in); + const char* path = full.c_str(); return (::unlink(path) == 0); } inline virtual bool rename(const char* from_path, const char* to_path) override { - return (::rename(from_path, to_path) == 0); + return (::rename(resolve(from_path).c_str(), resolve(to_path).c_str()) == 0); } - inline virtual bool mkdir(const char* path) override { + inline virtual bool mkdir(const char* path_in) override { + const std::string full = resolve(path_in); + const char* path = full.c_str(); struct stat st = {0}; if (::stat(path, &st) == 0) { if (S_ISDIR(st.st_mode)) { @@ -315,11 +324,13 @@ class PosixFileSystem : public microStore::FileSystem { } inline virtual bool rmdir(const char* path) override { - return (::rmdir(path) == 0); + return (::rmdir(resolve(path).c_str()) == 0); } - inline virtual size_t size(const char* path) override { + inline virtual size_t size(const char* path_in) override { + const std::string full = resolve(path_in); + const char* path = full.c_str(); struct stat st; if (stat(path, &st) != 0) { return 0; @@ -327,12 +338,16 @@ class PosixFileSystem : public microStore::FileSystem { return st.st_size; } - inline virtual bool isDirectory(const char* path) override { + inline virtual bool isDirectory(const char* path_in) override { + const std::string full = resolve(path_in); + const char* path = full.c_str(); struct stat st = {0}; return (::stat(path, &st) == 0 && S_ISDIR(st.st_mode)); } - virtual std::list listDirectory(const char* path, Callbacks::DirectoryListing callback = nullptr) override { + virtual std::list listDirectory(const char* path_in, Callbacks::DirectoryListing callback = nullptr) override { + const std::string full = resolve(path_in); + const char* path = full.c_str(); std::list files; DIR *dir = ::opendir(path); if (dir == NULL) { @@ -369,7 +384,16 @@ class PosixFileSystem : public microStore::FileSystem { } private: - const char* _basepath = ""; + std::string resolve(const char* path) const { + if (path == nullptr || _basepath.empty() || path[0] == '/') { + return path ? path : ""; + } + if (path[0] == '.' && path[1] == '/') { + path += 2; + } + return _basepath + "/" + path; + } + std::string _basepath; }; }; diff --git a/test/test_posix_fs/test_posix_fs.cpp b/test/test_posix_fs/test_posix_fs.cpp index 68d2bff..2e756c7 100644 --- a/test/test_posix_fs/test_posix_fs.cpp +++ b/test/test_posix_fs/test_posix_fs.cpp @@ -6,6 +6,7 @@ #include #include #include +#include #include #include @@ -37,6 +38,58 @@ void test_stdio_rmdir_reports_result() { check_rmdir(microStore::Adapters::StdioFileSystem()); } +/* ---- Base path ---- */ + +static void write_file(microStore::FileSystem& fs, const char* path) { + microStore::File f = fs.open(path, microStore::File::ModeWrite, true); + TEST_ASSERT_TRUE((bool)f); + TEST_ASSERT_EQUAL(4, f.write("test", 4)); + f.close(); +} + +void test_posix_relative_paths_resolve_against_basepath() { + microStore::Adapters::PosixFileSystem fs(tmp.c_str()); + TEST_ASSERT_TRUE(fs.mkdir("./store/")); + TEST_ASSERT_TRUE(is_dir(tmp + "/store")); + TEST_ASSERT_TRUE(fs.isDirectory("store")); + + write_file(fs, "./store/a"); + TEST_ASSERT_EQUAL(0, ::access((tmp + "/store/a").c_str(), F_OK)); + TEST_ASSERT_TRUE(fs.exists("store/a")); + TEST_ASSERT_EQUAL(4, fs.size("./store/a")); + TEST_ASSERT_EQUAL(1, fs.listDirectory("./store/").size()); + + TEST_ASSERT_TRUE(fs.rename("./store/a", "./store/b")); + TEST_ASSERT_EQUAL(0, ::access((tmp + "/store/b").c_str(), F_OK)); + TEST_ASSERT_TRUE(fs.remove("./store/b")); + TEST_ASSERT_FALSE(fs.exists("./store/b")); + TEST_ASSERT_TRUE(fs.rmdir("./store/")); + TEST_ASSERT_FALSE(is_dir(tmp + "/store")); +} + +void test_posix_absolute_paths_ignore_basepath() { + microStore::Adapters::PosixFileSystem fs("/nonexistent-base"); + const std::string file = tmp + "/abs"; + write_file(fs, file.c_str()); + TEST_ASSERT_TRUE(fs.exists(file.c_str())); +} + +void test_posix_basepath_is_copied() { + std::string* base = new std::string(tmp); + microStore::Adapters::PosixFileSystem fs(base->c_str()); + base->assign(base->size(), 'x'); + delete base; + write_file(fs, "./copied"); + TEST_ASSERT_EQUAL(0, ::access((tmp + "/copied").c_str(), F_OK)); +} + +void test_posix_store_lives_under_basepath() { + microStore::Adapters::PosixFileSystem fs(tmp.c_str()); + microStore::FileStore store; + TEST_ASSERT_TRUE(store.init(fs, "./kv/")); + TEST_ASSERT_TRUE(is_dir(tmp + "/kv")); +} + /* ---- Main ---- */ void setUp() { @@ -54,6 +107,10 @@ int runUnityTests(void) { UNITY_BEGIN(); RUN_TEST(test_posix_rmdir_reports_result); RUN_TEST(test_stdio_rmdir_reports_result); + RUN_TEST(test_posix_relative_paths_resolve_against_basepath); + RUN_TEST(test_posix_absolute_paths_ignore_basepath); + RUN_TEST(test_posix_basepath_is_copied); + RUN_TEST(test_posix_store_lives_under_basepath); return UNITY_END(); }