Fixing file deployent (nanoff or Visual Studio) for ChibiOS targets

targets/ChibiOS/_common/targetHAL_StorageOperation.cpp

@@ -20,44 +20,70 @@
 
 #include <hal_littlefs.h>
 
-// Function to create all necessary intermediate directories with LittleFS
-int create_directories(lfs_t *lfs, const char *path)
+// Must match FileAttributes::FileAttributes_Normal (managed System.IO.FileAttributes) without pulling in
+// the full CLR runtime headers (nf_sys_io_filesystem.h) into this HAL-layer file.
+static const uint32_t c_FileAttributesNormal = 128;
+
+// Function to create all necessary intermediate directories with LittleFS.
+//
+// IMPORTANT: 'path' must already be a relative path (no drive prefix, no leading '/') using '/' as the
+// separator - this is guaranteed by the normalization done in HAL_StorageOperation() before this is called.
+// This function mutates 'path' in place temporarily (splitting on '/') but always restores it before
+// returning. It intentionally avoids any local stack buffer: this runs deep in the wire-protocol/debugger
+// call chain (WP_Message -> Monitor_StorageOperation -> HAL_StorageOperation -> create_directories), which
+// may run on a thread with a small stack; an extra local buffer here previously caused a stack overflow
+// (silent hang, no reply sent back) when deploying files inside a subfolder.
+static int create_directories(lfs_t *lfs, char *path)
 {
-    char temp[256];
     char *pos = NULL;
-    size_t len = 0;
     int res = 0;
+    struct lfs_info info;
 
-    len = hal_strlen_s(path);
-
-    snprintf(temp, sizeof(temp), "%s", path);
-    if (temp[len - 1] == '/')
+    if (*path == '\0')
     {
-        temp[len - 1] = 0;
+        return 0;
     }
 
-    for (pos = temp + 1; *pos; pos++)
+    for (pos = path; *pos; pos++)
     {
         if (*pos == '/')
         {
-            *pos = 0;
-            res = lfs_mkdir(lfs, temp);
-            if (res == LFS_ERR_OK)
+            *pos = '\0';
+            res = lfs_mkdir(lfs, path);
+            *pos = '/';
+
+            if (res == LFS_ERR_EXIST)
             {
+                // Existing entry must be a directory.
+                *pos = '\0';
+                bool isDir = (lfs_stat(lfs, path, &info) == LFS_ERR_OK && info.type == LFS_TYPE_DIR);
                 *pos = '/';
-                continue;
+
+                if (!isDir)
+                {
+                    return -1;
+                }
             }
-            else if (res != LFS_ERR_EXIST)
+            else if (res != LFS_ERR_OK)
             {
                 return -1;
             }
         }
     }
 
     // Create the final directory
-    res = lfs_mkdir(lfs, temp);
+    res = lfs_mkdir(lfs, path);
+    if (res == LFS_ERR_OK)
+    {
+        return 0;
+    }
+
+    if (res == LFS_ERR_EXIST)
+    {
+        return (lfs_stat(lfs, path, &info) == LFS_ERR_OK && info.type == LFS_TYPE_DIR) ? 0 : -1;
+    }
 
-    return res == LFS_ERR_OK || res == LFS_ERR_EXIST ? 0 : -1;
+    return -1;
 }
 
 uint32_t HAL_StorageOperation(
@@ -97,16 +123,52 @@ uint32_t HAL_StorageOperation(
     // Just making sure it's properly 0 terminated
     storageName[nameLength] = '\0';
 
+    // Normalize path for LittleFS.
+    // 1) Strip optional drive prefix (e.g. "I:\\" or "I:/")
+    // 2) Strip any leading path separator(s), regardless of whether a drive prefix was present
+    //    (handles "I:\foo", "I:/foo", "\foo", "/foo", etc.) so LittleFS always receives a path
+    //    that doesn't start with a separator - otherwise create_directories() would end up
+    //    calling lfs_mkdir() with an empty path for the leading separator and fail.
+    // 3) Convert Windows separators to '/'
+    char *lfsPath = storageName;
+
+    if (nameLength >= 2 && storageName[1] == ':')
+    {
+        lfsPath = storageName + 2;
+    }
+
+    while (*lfsPath == '\\' || *lfsPath == '/')
+    {
+        lfsPath++;
+    }
+
+    for (char *p = lfsPath; *p; p++)
+    {
+        if (*p == '\\')
+        {
+            *p = '/';
+        }
+    }
+
     //... and pointer to the littlefs instance
     lfsDrive = hal_lfs_get_fs_from_index(driveIndex);
 
     if (operation == StorageOperation_Monitor::StorageOperation_Write)
     {
         memset(&lfsFile, 0, sizeof(lfsFile));
 
-        // Extract directory path from file path
+        // Extract directory path from file path.
+        // Reject paths that don't fit dir_path instead of silently truncating them - truncating here
+        // would create directories for a shorter/different path than the one lfs_file_opencfg() below
+        // actually opens (using the full, untruncated lfsPath), leaving the real parent directory missing.
         char dir_path[256];
-        snprintf(dir_path, sizeof(dir_path), "%s", storageName);
+        if (hal_strlen_s(lfsPath) >= sizeof(dir_path))
+        {
+            errorCode = StorageOperationErrorCode::WriteError;
+            goto done;
+        }
+
+        snprintf(dir_path, sizeof(dir_path), "%s", lfsPath);
         char *last_slash = strrchr(dir_path, '/');
         if (last_slash != NULL)
         {
@@ -118,61 +180,146 @@ uint32_t HAL_StorageOperation(
             dir_path[0] = '\0';
         }
 
-        // Check if there is only one slash in the path
-        if (strchr(dir_path, '/') == strrchr(dir_path, '/'))
+        // Create all necessary intermediate directories
+        if (dir_path[0] != '\0')
         {
-            // Only one slash, skip creating the directory
-            dir_path[0] = '\0';
+            int32_t mkdirResult = create_directories(lfsDrive, dir_path);
+
+            if (mkdirResult != 0)
+            {
+                errorCode = StorageOperationErrorCode::WriteError;
+                goto done;
+            }
         }
 
-        // Create all necessary intermediate directories
-        if (dir_path[0] != '\0' && create_directories(lfsDrive, dir_path) != 0)
+        // Open/create the file in read mode.
+        // NOTE: must use lfs_file_opencfg with the NANO_LITTLEFS_ATTRIBUTE custom attribute, matching what
+        // LITTLEFS_FS_Driver::Open does. Without this attribute, managed File.GetAttributes()/File.Exists()
+        // will report EMPTY_ATTRIBUTE (0xFFFFFFFF) for this file - even though it exists on disk - because
+        // lfs_getattr() fails with LFS_ERR_NOATTR when the attribute was never written.
+        uint32_t nanoAttributes = c_FileAttributesNormal;
+        struct lfs_attr attr = {NANO_LITTLEFS_ATTRIBUTE, &nanoAttributes, NANO_LITTLEFS_ATTRIBUTE_SIZE};
+        struct lfs_file_config fileConfig = {
+            .buffer = NULL,
+            .attrs = &attr,
+            .attr_count = 1,
+        };
+
+        int32_t openResult = lfs_file_opencfg(lfsDrive, &lfsFile, lfsPath, LFS_O_RDWR | LFS_O_CREAT, &fileConfig);
+        if (openResult != LFS_ERR_OK)
         {
             errorCode = StorageOperationErrorCode::WriteError;
+
             goto done;
         }
 
-        // Open/create the file in read mode
-        if (lfs_file_open(lfsDrive, &lfsFile, storageName, LFS_O_RDWR | LFS_O_CREAT) != LFS_ERR_OK)
+        // persist the custom attribute immediately, so it survives even if the write below fails/
+        if (lfs_file_sync(lfsDrive, &lfsFile) != LFS_ERR_OK)
         {
             errorCode = StorageOperationErrorCode::WriteError;
-
+            lfs_file_close(lfsDrive, &lfsFile);
             goto done;
         }
 
-        if (lfs_file_write(lfsDrive, &lfsFile, (data + nameLength), dataLength) != (lfs_ssize_t)dataLength)
+        lfs_ssize_t writeResult = lfs_file_write(lfsDrive, &lfsFile, (data + nameLength), dataLength);
+        if (writeResult != (lfs_ssize_t)dataLength)
         {
             // failed to write expected number of bytes
             errorCode = StorageOperationErrorCode::WriteError;
         }
 
-        // close file
-        lfs_file_close(lfsDrive, &lfsFile);
+        // close file - a failed close can mean the write was never actually committed, even if
+        // lfs_file_write() above reported success, so this must also be treated as a failure.
+        if (lfs_file_close(lfsDrive, &lfsFile) != LFS_ERR_OK)
+        {
+            errorCode = StorageOperationErrorCode::WriteError;
+        }
     }
     else if (operation == StorageOperation_Monitor::StorageOperation_Append)
     {
-        // Open/create the file in read mode
-        if (lfs_file_open(lfsDrive, &lfsFile, storageName, LFS_O_RDWR | LFS_O_APPEND) != LFS_ERR_OK)
+        memset(&lfsFile, 0, sizeof(lfsFile));
+
+        // Extract directory path from file path.
+        // Reject paths that don't fit dir_path instead of silently truncating them - see the comment
+        // in the Write branch above for why truncation here would be incorrect.
+        char dir_path[256];
+        if (hal_strlen_s(lfsPath) >= sizeof(dir_path))
+        {
+            errorCode = StorageOperationErrorCode::WriteError;
+            goto done;
+        }
+
+        snprintf(dir_path, sizeof(dir_path), "%s", lfsPath);
+        char *last_slash = strrchr(dir_path, '/');
+        if (last_slash != NULL)
+        {
+            *last_slash = '\0';
+        }
+        else
+        {
+            // No directory part, file is in the root
+            dir_path[0] = '\0';
+        }
+
+        // Create all necessary intermediate directories
+        if (dir_path[0] != '\0')
+        {
+            int32_t mkdirResult = create_directories(lfsDrive, dir_path);
+
+            if (mkdirResult != 0)
+            {
+                errorCode = StorageOperationErrorCode::WriteError;
+                goto done;
+            }
+        }
+
+        // Open/create the file in read mode.
+        // Same custom-attribute requirement as the Write branch above - see the comment there.
+        uint32_t nanoAttributes = c_FileAttributesNormal;
+        struct lfs_attr attr = {NANO_LITTLEFS_ATTRIBUTE, &nanoAttributes, NANO_LITTLEFS_ATTRIBUTE_SIZE};
+        struct lfs_file_config fileConfig = {
+            .buffer = NULL,
+            .attrs = &attr,
+            .attr_count = 1,
+        };
+
+        int32_t openResult =
+            lfs_file_opencfg(lfsDrive, &lfsFile, lfsPath, LFS_O_RDWR | LFS_O_APPEND | LFS_O_CREAT, &fileConfig);
+        if (openResult != LFS_ERR_OK)
         {
             errorCode = StorageOperationErrorCode::WriteError;
 
             goto done;
         }
 
+        // persist the custom attribute immediately, so it survives even if the write below fails
+        if (lfs_file_sync(lfsDrive, &lfsFile) != LFS_ERR_OK)
+        {
+            errorCode = StorageOperationErrorCode::WriteError;
+            lfs_file_close(lfsDrive, &lfsFile);
+            goto done;
+        }
+
         // append more data
-        if (lfs_file_write(lfsDrive, &lfsFile, (data + nameLength), dataLength) != (lfs_ssize_t)dataLength)
+        lfs_ssize_t writeResult = lfs_file_write(lfsDrive, &lfsFile, (data + nameLength), dataLength);
+        if (writeResult != (lfs_ssize_t)dataLength)
         {
             // failed to write expected number of bytes
             errorCode = StorageOperationErrorCode::WriteError;
         }
 
-        // close file
-        lfs_file_close(lfsDrive, &lfsFile);
+        // close file - a failed close can mean the write was never actually committed, even if
+        // lfs_file_write() above reported success, so this must also be treated as a failure.
+        if (lfs_file_close(lfsDrive, &lfsFile) != LFS_ERR_OK)
+        {
+            errorCode = StorageOperationErrorCode::WriteError;
+        }
     }
     else if (operation == StorageOperation_Monitor::StorageOperation_Delete)
     {
         // remove the file
-        if (lfs_remove(lfsDrive, storageName) != LFS_ERR_OK)
+        int32_t removeResult = lfs_remove(lfsDrive, lfsPath);
+        if (removeResult != LFS_ERR_OK && removeResult != LFS_ERR_NOENT)
         {
             errorCode = StorageOperationErrorCode::DeleteError;
         }