diff --git a/targets/ChibiOS/_common/targetHAL_StorageOperation.cpp b/targets/ChibiOS/_common/targetHAL_StorageOperation.cpp index a826e459fb..f444d9bc2a 100644 --- a/targets/ChibiOS/_common/targetHAL_StorageOperation.cpp +++ b/targets/ChibiOS/_common/targetHAL_StorageOperation.cpp @@ -20,34 +20,51 @@ #include -// 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; } @@ -55,9 +72,18 @@ int create_directories(lfs_t *lfs, const char *path) } // 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,6 +123,33 @@ 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); @@ -104,9 +157,18 @@ uint32_t HAL_StorageOperation( { 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; }