FileSystem: a file system is named only at the start of a filename

components/core/FileSystem/FileSystem.cpp

@@ -11,6 +11,7 @@
 #include "FileSystemWorker.h"
 #include <algorithm>
 #include <strings.h>
+#include <string.h>
 #include "RaftUtils.h"
 #include "Logger.h"
 
@@ -87,12 +88,27 @@ RaftMutex& FileSystem::fsMutex(const char* nameOfFS) const
 
 RaftMutex& FileSystem::fsMutex(const String& nameOfFS, const String& filename) const
 {
-    // A file system named in the filename is the one used (see getFilePath())
-    if (filename.indexOf(SD_FILE_SYSTEM_PATH_ELEMENT) >= 0)
-        return _sdFsMutex;
-    if (filename.indexOf(LOCAL_FILE_SYSTEM_PATH_ELEMENT) >= 0)
-        return _localFsMutex;
-    return fsMutex(nameOfFS.c_str());
+    // A file system named at the start of the filename is the one used (see getFilePath())
+    const char* pNamedFS = fileSystemNamedIn(filename);
+    return fsMutex(pNamedFS ? pNamedFS : nameOfFS.c_str());
+}
+
+/////////////////////////////////////////////////////////////////////////////////////////////////////////////////
+// The file system a filename names at its start (after an optional /), or nullptr
+/////////////////////////////////////////////////////////////////////////////////////////////////////////////////
+
+const char* FileSystem::fileSystemNamedIn(const String& filename)
+{
+    const char* pName = filename.c_str();
+    if (*pName == '/')
+        pName++;
+    for (const char* pFS : { LOCAL_FILE_SYSTEM_NAME, SD_FILE_SYSTEM_NAME })
+    {
+        size_t len = strlen(pFS);
+        if ((strncmp(pName, pFS, len) == 0) && ((pName[len] == '/') || (pName[len] == '\0')))
+            return pFS;
+    }
+    return nullptr;
 }
 
 RaftMutex& FileSystem::pathMutex(const char* path) const
@@ -680,8 +696,8 @@ bool FileSystem::checkFileSystem(const String& fileSystemStr, String& fsName) co
 
 String FileSystem::getFilePath(const String& nameOfFS, const String& filename) const
 {
-    // Check if filename already contains file system
-    if ((filename.indexOf(LOCAL_FILE_SYSTEM_PATH_ELEMENT) >= 0) || (filename.indexOf(SD_FILE_SYSTEM_PATH_ELEMENT) >= 0))
+    // A filename that starts with a file system name is already a full path
+    if (fileSystemNamedIn(filename))
         return (filename.startsWith("/") ? filename : ("/" + filename));
 
 #ifdef __linux__

components/core/FileSystem/FileSystem.h

@@ -258,6 +258,12 @@ class FileSystem
     // File system partition name
     String _fsPartitionName;
 
+public:
+    // The file system a filename names at its start - "local/...", "/local/...", "sd/..." or "/sd/..." (or just the
+    // file system's name) - or nullptr if it doesn't name one. Only the start counts: "logs/sd/x.csv" is a file
+    // in a folder called sd
+    static const char* fileSystemNamedIn(const String& filename);
+
 private:
     bool checkFileSystem(const String& fileSystemStr, String& fsName) const;
     String getFilePath(const String& nameOfFS, const String& filename) const;
@@ -290,11 +296,9 @@ class FileSystem
     // File system name
     static constexpr const char* LOCAL_FILE_SYSTEM_NAME = "local";
     static constexpr const char* LOCAL_FILE_SYSTEM_BASE_PATH = "/local";
-    static constexpr const char* LOCAL_FILE_SYSTEM_PATH_ELEMENT = "local/";
     static constexpr const char* LOCAL_FILE_SYSTEM_ALT_NAME = "spiffs";
     static constexpr const char* SD_FILE_SYSTEM_NAME = "sd";
     static constexpr const char* SD_FILE_SYSTEM_BASE_PATH = "/sd";
-    static constexpr const char* SD_FILE_SYSTEM_PATH_ELEMENT = "sd/";
     static constexpr const char* LOCAL_FILE_SYSTEM_PARTITION_LABEL = "fs";
     static constexpr const char* LOCAL_FILE_SYSTEM_PARTITION_LABEL_ALT = "spiffs";
 

linux_unit_tests/Makefile

@@ -114,12 +114,13 @@ SOURCES_test_file_upload_okto = test_file_upload_okto.cpp $(COMMS_SOURCES) $(COM
 SOURCES_test_file_upload_background = test_file_upload_background.cpp $(COMMS_SOURCES) $(COMMON_SOURCES)
 SOURCES_test_thread_safe_queue = test_thread_safe_queue.cpp $(COMMON_SOURCES)
 SOURCES_test_simple_buffer = test_simple_buffer.cpp $(CORE)/MiniHDLC/MiniHDLC.cpp $(COMMON_SOURCES)
+SOURCES_test_file_system_paths = test_file_system_paths.cpp $(COMMS_SOURCES) $(COMMON_SOURCES)
 SOURCES_test_mini_hdlc = test_mini_hdlc.cpp $(CORE)/MiniHDLC/MiniHDLC.cpp $(COMMON_SOURCES)
 SOURCES_test_espnow_config = test_espnow_config.cpp $(ESPNOW_DIR)/ESPNowConfig.cpp $(COMMON_SOURCES)
 
 # Tests run by "make check"
 TESTS = test_raft_json test_raft_utils test_device_names test_device_settings test_device_type_settings test_device_data_subscribers test_device_type_records \
-  test_device_type_table test_device_type_grammar test_device_type_policy test_device_type_file test_catalogue_index test_runtime_decoder test_bus_ident test_bus_scan_schedule test_device_poll_timing test_device_poll_scheduler test_poll_plan test_protocol_exchange test_file_upload_okto test_file_upload_background test_mini_hdlc test_thread_safe_queue test_simple_buffer
+  test_device_type_table test_device_type_grammar test_device_type_policy test_device_type_file test_catalogue_index test_runtime_decoder test_bus_ident test_bus_scan_schedule test_device_poll_timing test_device_poll_scheduler test_poll_plan test_protocol_exchange test_file_upload_okto test_file_upload_background test_mini_hdlc test_thread_safe_queue test_simple_buffer test_file_system_paths
 
 # Extra device type catalogues for test_device_type_records to sweep, e.g.
 #   make check DEV_TYPE_CATALOGUES="../../RaftI2C/linux_unit_tests/TestDevTypeRecs.json"

linux_unit_tests/test_file_system_paths.cpp

@@ -0,0 +1,47 @@
+/////////////////////////////////////////////////////////////////////////////////////////////////////////////////
+//
+// FileSystem path tests
+//
+// A filename names a file system only at its start ("local/...", "/sd/..."): it then picks the path and the lock.
+// "logs/sd/x.csv" is a file in a folder called sd on whichever file system was given.
+//
+/////////////////////////////////////////////////////////////////////////////////////////////////////////////////
+
+#include <string.h>
+#include "testhdr.h"
+#include "FileSystem.h"
+
+static bool namedIs(const char* filename, const char* expected)
+{
+    const char* pNamed = FileSystem::fileSystemNamedIn(filename);
+    if (!expected)
+        return pNamed == nullptr;
+    return pNamed && (strcmp(pNamed, expected) == 0);
+}
+
+static void testNamedAtStart()
+{
+    printf("Running testNamedAtStart...\n");
+    TEST_ASSERT(namedIs("local/a.txt", "local"), "local/a.txt");
+    TEST_ASSERT(namedIs("/local/web/index.html", "local"), "/local/...");
+    TEST_ASSERT(namedIs("sd/a.txt", "sd"), "sd/a.txt");
+    TEST_ASSERT(namedIs("/sd/images/x.jpg", "sd"), "/sd/...");
+    TEST_ASSERT(namedIs("/local", "local") && namedIs("sd", "sd"), "the name alone");
+}
+
+static void testNotNamed()
+{
+    printf("Running testNotNamed...\n");
+    TEST_ASSERT(namedIs("logs/sd/x.csv", nullptr), "sd/ in the middle");
+    TEST_ASSERT(namedIs("/data/local/x", nullptr), "local/ in the middle");
+    TEST_ASSERT(namedIs("backup/nonlocal/x", nullptr), "nonlocal/");
+    TEST_ASSERT(namedIs("sdcard/x", nullptr) && namedIs("/localdata/x", nullptr), "names starting with a file system name");
+    TEST_ASSERT(namedIs("a.txt", nullptr) && namedIs("/a.txt", nullptr) && namedIs("", nullptr), "no file system");
+}
+
+int main()
+{
+    testNamedAtStart();
+    testNotNamed();
+    return testSummary("test_file_system_paths");
+}