From c5cf2ce53f65b40d6d4251419dcd54b1223b4888 Mon Sep 17 00:00:00 2001 From: Willy-JL <49810075+Willy-JL@users.noreply.github.com> Date: Wed, 31 May 2023 15:43:11 +0100 Subject: [PATCH] Fix brick on rename across filesystems --- .../services/storage/storage_external_api.c | 26 ++++++++-- .../services/storage/storage_processing.c | 51 ++++++++----------- 2 files changed, 45 insertions(+), 32 deletions(-) diff --git a/applications/services/storage/storage_external_api.c b/applications/services/storage/storage_external_api.c index 0b4587926..bfedb3e1e 100644 --- a/applications/services/storage/storage_external_api.c +++ b/applications/services/storage/storage_external_api.c @@ -446,7 +446,18 @@ FS_Error storage_common_rename(Storage* storage, const char* old_path, const cha S_API_MESSAGE(StorageCommandCommonRename); S_API_EPILOGUE; - return S_RETURN_ERROR; + FS_Error ret = S_RETURN_ERROR; + + if(ret == FSE_NOT_IMPLEMENTED) { + // Different filesystems, use copy + remove + ret = storage_common_copy(storage, old_path, new_path); + if(ret == FSE_OK) { + if(!storage_simply_remove_recursive(storage, old_path)) { + ret = FSE_INTERNAL; + } + } + } + return ret; } FS_Error storage_common_move(Storage* storage, const char* old_path, const char* new_path) { @@ -458,7 +469,7 @@ FS_Error storage_common_move(Storage* storage, const char* old_path, const char* return FSE_INVALID_NAME; } - if(storage_common_exists(storage, new_path)) { + if(storage_file_exists(storage, new_path)) { FS_Error error = storage_common_remove(storage, new_path); if(error != FSE_OK) { return error; @@ -475,7 +486,16 @@ FS_Error storage_common_move(Storage* storage, const char* old_path, const char* S_API_MESSAGE(StorageCommandCommonRename); S_API_EPILOGUE; - return S_RETURN_ERROR; + FS_Error ret = S_RETURN_ERROR; + + if(ret == FSE_NOT_IMPLEMENTED) { + // Different filesystems, use copy + remove + ret = storage_common_copy(storage, old_path, new_path); + if(ret == FSE_OK) { + ret = storage_simply_remove_recursive(storage, old_path); + } + } + return ret; } static FS_Error diff --git a/applications/services/storage/storage_processing.c b/applications/services/storage/storage_processing.c index 523400184..0eaf23b18 100644 --- a/applications/services/storage/storage_processing.c +++ b/applications/services/storage/storage_processing.c @@ -356,47 +356,32 @@ static FS_Error storage_process_common_remove(Storage* app, FuriString* path) { StorageData* storage; FS_Error ret = storage_get_data(app, path, &storage); - do { + if(ret == FSE_OK) { if(storage_path_already_open(path, storage)) { - ret = FSE_ALREADY_OPEN; - break; + return FSE_ALREADY_OPEN; } storage_data_timestamp(storage); FS_CALL(storage, common.remove(storage, cstr_path_without_vfs_prefix(path))); - } while(false); + } return ret; } static FS_Error storage_process_common_rename(Storage* app, FuriString* old, FuriString* new) { - FS_Error ret; - // Paths are already resolved, no aliases - if(strncmp(furi_string_get_cstr(old), furi_string_get_cstr(new), STORAGE_PATH_PREFIX_LEN)) { - // Different filesystems, use copy + remove - ret = storage_common_copy(app, furi_string_get_cstr(old), furi_string_get_cstr(new)); - if(ret == FSE_OK) { - if(!storage_simply_remove_recursive(app, furi_string_get_cstr(old))) { - ret = FSE_INTERNAL; - } + StorageData* storage; + FS_Error ret = storage_get_data(app, old, &storage); + + if(ret == FSE_OK) { + if(storage_path_already_open(old, storage)) { + return FSE_ALREADY_OPEN; } - } else { - // Same filesystem, use rename - StorageData* storage; - ret = storage_get_data(app, old, &storage); - do { - if(storage_path_already_open(old, storage)) { - ret = FSE_ALREADY_OPEN; - break; - } - - storage_data_timestamp(storage); - FS_CALL( - storage, - common.rename( - storage, cstr_path_without_vfs_prefix(old), cstr_path_without_vfs_prefix(new))); - } while(false); + storage_data_timestamp(storage); + FS_CALL( + storage, + common.rename( + storage, cstr_path_without_vfs_prefix(old), cstr_path_without_vfs_prefix(new))); } return ret; @@ -653,6 +638,14 @@ void storage_process_message_internal(Storage* app, StorageMessage* message) { storage_process_alias(app, opath, message->data->rename.thread_id, false); path = furi_string_alloc_set(message->data->rename.new); storage_process_alias(app, path, message->data->rename.thread_id, false); + // Paths are resolved, no aliases + if(strncmp( + furi_string_get_cstr(opath), furi_string_get_cstr(path), STORAGE_PATH_PREFIX_LEN)) { + // Different filesystems, return to caller + message->return_data->error_value = FSE_NOT_IMPLEMENTED; + break; + } + // Same filesystem, use rename message->return_data->error_value = storage_process_common_rename(app, opath, path); break; case StorageCommandCommonMkDir: