From e0fa360640b4e8d3c1f419bec8c8425895bb11be Mon Sep 17 00:00:00 2001 From: Willy-JL <49810075+Willy-JL@users.noreply.github.com> Date: Tue, 28 Nov 2023 00:00:40 +0000 Subject: [PATCH] Fix rename/move API, now rename/rename_safe (#468) On OFW "rename" acts like "move", it replaces the destination XFW had an extra "move" like that, and "rename" errored if dest exists Now for compatibility "rename" acts as OFW, and new "rename_safe" errors Tweaked all usages to work properly Decided for CLI and RPC to use "rename_safe" so user cant lose files --- .../main/archive/helpers/archive_favorites.c | 24 +++++------ .../main/archive/helpers/archive_files.c | 2 +- applications/main/bad_kb/bad_kb_app.c | 2 +- applications/services/rpc/rpc_storage.c | 2 +- applications/services/storage/storage.h | 13 +++--- applications/services/storage/storage_cli.c | 2 +- .../services/storage/storage_external_api.c | 41 +++++++++---------- furi/flipper.c | 6 +-- targets/f7/api_symbols.csv | 2 +- 9 files changed, 47 insertions(+), 47 deletions(-) diff --git a/applications/main/archive/helpers/archive_favorites.c b/applications/main/archive/helpers/archive_favorites.c index db0700386..fad900e69 100644 --- a/applications/main/archive/helpers/archive_favorites.c +++ b/applications/main/archive/helpers/archive_favorites.c @@ -104,9 +104,9 @@ static bool archive_favourites_rescan() { furi_string_free(buffer); storage_file_close(file); - if(storage_common_move(storage, ARCHIVE_FAV_TEMP_PATH, ARCHIVE_FAV_PATH) == FSE_NOT_EXIST) { - storage_common_remove(storage, ARCHIVE_FAV_PATH); - } + storage_common_remove(storage, ARCHIVE_FAV_PATH); + storage_common_rename(storage, ARCHIVE_FAV_TEMP_PATH, ARCHIVE_FAV_PATH); + storage_common_remove(storage, ARCHIVE_FAV_TEMP_PATH); storage_file_free(file); furi_record_close(RECORD_STORAGE); @@ -203,9 +203,9 @@ bool archive_favorites_delete(const char* format, ...) { furi_string_free(filename); storage_file_close(file); - if(storage_common_move(fs_api, ARCHIVE_FAV_TEMP_PATH, ARCHIVE_FAV_PATH) == FSE_NOT_EXIST) { - storage_common_remove(fs_api, ARCHIVE_FAV_PATH); - } + storage_common_remove(fs_api, ARCHIVE_FAV_PATH); + storage_common_rename(fs_api, ARCHIVE_FAV_TEMP_PATH, ARCHIVE_FAV_PATH); + storage_common_remove(fs_api, ARCHIVE_FAV_TEMP_PATH); storage_file_free(file); furi_record_close(RECORD_STORAGE); @@ -282,9 +282,9 @@ bool archive_favorites_rename(const char* src, const char* dst) { furi_string_free(path); storage_file_close(file); - if(storage_common_move(fs_api, ARCHIVE_FAV_TEMP_PATH, ARCHIVE_FAV_PATH) == FSE_NOT_EXIST) { - storage_common_remove(fs_api, ARCHIVE_FAV_PATH); - } + storage_common_remove(fs_api, ARCHIVE_FAV_PATH); + storage_common_rename(fs_api, ARCHIVE_FAV_TEMP_PATH, ARCHIVE_FAV_PATH); + storage_common_remove(fs_api, ARCHIVE_FAV_TEMP_PATH); storage_file_free(file); furi_record_close(RECORD_STORAGE); @@ -310,9 +310,9 @@ void archive_favorites_save(void* context) { archive_file_append(ARCHIVE_FAV_TEMP_PATH, "%s\n", furi_string_get_cstr(item->path)); } - if(storage_common_move(fs_api, ARCHIVE_FAV_TEMP_PATH, ARCHIVE_FAV_PATH) == FSE_NOT_EXIST) { - storage_common_remove(fs_api, ARCHIVE_FAV_PATH); - } + storage_common_remove(fs_api, ARCHIVE_FAV_PATH); + storage_common_rename(fs_api, ARCHIVE_FAV_TEMP_PATH, ARCHIVE_FAV_PATH); + storage_common_remove(fs_api, ARCHIVE_FAV_TEMP_PATH); storage_file_free(file); furi_record_close(RECORD_STORAGE); diff --git a/applications/main/archive/helpers/archive_files.c b/applications/main/archive/helpers/archive_files.c index e839fc772..3e78c0d1b 100644 --- a/applications/main/archive/helpers/archive_files.c +++ b/applications/main/archive/helpers/archive_files.c @@ -178,7 +178,7 @@ FS_Error archive_copy_rename_file_or_dir( if(copy) { error = storage_common_copy(fs_api, src_path, furi_string_get_cstr(dst_path)); } else { - error = storage_common_rename(fs_api, src_path, furi_string_get_cstr(dst_path)); + error = storage_common_rename_safe(fs_api, src_path, furi_string_get_cstr(dst_path)); } } furi_record_close(RECORD_STORAGE); diff --git a/applications/main/bad_kb/bad_kb_app.c b/applications/main/bad_kb/bad_kb_app.c index 1c3156bcd..4ee226481 100644 --- a/applications/main/bad_kb/bad_kb_app.c +++ b/applications/main/bad_kb/bad_kb_app.c @@ -120,7 +120,7 @@ BadKbApp* bad_kb_app_alloc(char* arg) { } Storage* storage = furi_record_open(RECORD_STORAGE); - storage_common_rename(storage, EXT_PATH("badusb"), BAD_KB_APP_BASE_FOLDER); + storage_common_rename_safe(storage, EXT_PATH("badusb"), BAD_KB_APP_BASE_FOLDER); storage_simply_mkdir(storage, BAD_KB_APP_BASE_FOLDER); furi_record_close(RECORD_STORAGE); diff --git a/applications/services/rpc/rpc_storage.c b/applications/services/rpc/rpc_storage.c index 825e6df82..5fd232642 100644 --- a/applications/services/rpc/rpc_storage.c +++ b/applications/services/rpc/rpc_storage.c @@ -651,7 +651,7 @@ static void rpc_system_storage_rename_process(const PB_Main* request, void* cont Storage* fs_api = furi_record_open(RECORD_STORAGE); if(path_contains_only_ascii(request->content.storage_rename_request.new_path)) { - FS_Error error = storage_common_rename( + FS_Error error = storage_common_rename_safe( fs_api, request->content.storage_rename_request.old_path, request->content.storage_rename_request.new_path); diff --git a/applications/services/storage/storage.h b/applications/services/storage/storage.h index 7449b7c60..bcca46e30 100644 --- a/applications/services/storage/storage.h +++ b/applications/services/storage/storage.h @@ -312,6 +312,7 @@ FS_Error storage_common_remove(Storage* storage, const char* path); * @brief Rename a file or a directory. * * The file or the directory must NOT be open. + * Will overwrite the destination file if it already exists. * * Renaming a regular file to itself does nothing and always succeeds. * Renaming a directory to itself or to a subdirectory of itself always fails. @@ -324,20 +325,20 @@ FS_Error storage_common_remove(Storage* storage, const char* path); FS_Error storage_common_rename(Storage* storage, const char* old_path, const char* new_path); /** - * @brief Move a file or a directory. + * @brief Rename a file or a directory. * * The file or the directory must NOT be open. - * Will overwrite the destination file if it already exists. + * Will error FSE_EXIST if the destination file already exists. * - * Moving a regular file to itself does nothing and always succeeds. - * Moving a directory to itself or to a subdirectory of itself always fails. + * Renaming a regular file to itself does nothing and always succeeds. + * Renaming a directory to itself or to a subdirectory of itself always fails. * * @param storage pointer to a storage API instance. * @param old_path pointer to a zero-terminated string containing the source path. * @param new_path pointer to a zero-terminated string containing the destination path. - * @return FSE_OK if the file or directory has been successfully moved, any other error code on failure. + * @return FSE_OK if the file or directory has been successfully renamed, any other error code on failure. */ -FS_Error storage_common_move(Storage* storage, const char* old_path, const char* new_path); +FS_Error storage_common_rename_safe(Storage* storage, const char* old_path, const char* new_path); /** * @brief Copy the file to a new location. diff --git a/applications/services/storage/storage_cli.c b/applications/services/storage/storage_cli.c index ae35d6bbb..209e77c39 100644 --- a/applications/services/storage/storage_cli.c +++ b/applications/services/storage/storage_cli.c @@ -454,7 +454,7 @@ static void storage_cli_rename(Cli* cli, FuriString* old_path, FuriString* args) if(!args_read_probably_quoted_string_and_trim(args, new_path)) { storage_cli_print_usage(); } else { - FS_Error error = storage_common_rename( + FS_Error error = storage_common_rename_safe( api, furi_string_get_cstr(old_path), furi_string_get_cstr(new_path)); if(error != FSE_OK) { diff --git a/applications/services/storage/storage_external_api.c b/applications/services/storage/storage_external_api.c index 8cdf78617..0c6ff2295 100644 --- a/applications/services/storage/storage_external_api.c +++ b/applications/services/storage/storage_external_api.c @@ -479,12 +479,13 @@ FS_Error storage_common_rename(Storage* storage, const char* old_path, const cha break; } - if(storage_common_exists(storage, new_path)) { - error = FSE_EXIST; - break; - } - if(storage_dir_exists(storage, old_path)) { + // Cannot overwrite a file with a directory + if(storage_file_exists(storage, new_path)) { + error = FSE_INVALID_NAME; + break; + } + // Cannot rename a directory to itself or to a nested directory if(storage_common_equivalent_path(storage, old_path, new_path, true)) { error = FSE_INVALID_NAME; @@ -497,6 +498,10 @@ FS_Error storage_common_rename(Storage* storage, const char* old_path, const cha break; } + if(storage_file_exists(storage, new_path)) { + storage_common_remove(storage, new_path); + } + S_API_PROLOGUE; SAData data = { .rename = { @@ -525,7 +530,7 @@ FS_Error storage_common_rename(Storage* storage, const char* old_path, const cha return error; } -FS_Error storage_common_move(Storage* storage, const char* old_path, const char* new_path) { +FS_Error storage_common_rename_safe(Storage* storage, const char* old_path, const char* new_path) { FS_Error error; do { @@ -534,30 +539,24 @@ FS_Error storage_common_move(Storage* storage, const char* old_path, const char* break; } - if(storage_dir_exists(storage, old_path)) { - // Cannot overwrite a file with a directory - if(storage_file_exists(storage, new_path)) { - error = FSE_INVALID_NAME; - break; - } + if(storage_common_exists(storage, new_path)) { + error = FSE_EXIST; + break; + } - // Cannot move a directory to itself or to a nested directory + if(storage_dir_exists(storage, old_path)) { + // Cannot rename a directory to itself or to a nested directory if(storage_common_equivalent_path(storage, old_path, new_path, true)) { error = FSE_INVALID_NAME; break; } - // Moving a regular file to itself does nothing and always succeeds + // Renaming a regular file to itself does nothing and always succeeds } else if(storage_common_equivalent_path(storage, old_path, new_path, false)) { error = FSE_OK; break; } - if(storage_common_exists(storage, new_path)) { - error = storage_simply_remove_recursive(storage, new_path); - if(error != FSE_OK) break; - } - S_API_PROLOGUE; SAData data = { .rename = { @@ -756,7 +755,7 @@ static FS_Error if(error == FSE_OK) { if(file_info_is_dir(&fileinfo)) { if(!copy) { - error = storage_common_rename(storage, old_path, new_path); + error = storage_common_rename_safe(storage, old_path, new_path); } if(copy || error != FSE_OK) { error = storage_merge_recursive(storage, old_path, new_path, copy); @@ -814,7 +813,7 @@ static FS_Error stream_free(stream_from); stream_free(stream_to); } else { - error = storage_common_rename(storage, old_path, new_path_tmp); + error = storage_common_rename_safe(storage, old_path, new_path_tmp); } } } diff --git a/furi/flipper.c b/furi/flipper.c index 1ebaccdf1..0b33858fe 100644 --- a/furi/flipper.c +++ b/furi/flipper.c @@ -49,7 +49,7 @@ void flipper_migrate_files() { storage_common_remove(storage, INT_PATH(".passport.settings")); storage_common_remove(storage, INT_PATH(".region_data")); - // Migrate files + // Migrate files (use copy+remove to not overwrite dst but still delete src) FURI_LOG_I(TAG, "Migrate: Renames on external"); storage_common_copy(storage, ARCHIVE_FAV_OLD_PATH, ARCHIVE_FAV_PATH); storage_common_remove(storage, ARCHIVE_FAV_OLD_PATH); @@ -65,7 +65,7 @@ void flipper_migrate_files() { storage_common_remove(storage, POWER_SETTINGS_OLD_PATH); storage_common_copy(storage, BT_KEYS_STORAGE_OLD_PATH, BT_KEYS_STORAGE_PATH); storage_common_remove(storage, BT_KEYS_STORAGE_OLD_PATH); - storage_common_copy(storage, NOTIFICATION_SETTINGS_OLD_PATH, NOTIFICATION_SETTINGS_PATH); + // storage_common_copy(storage, NOTIFICATION_SETTINGS_OLD_PATH, NOTIFICATION_SETTINGS_PATH); // Not compatible anyway storage_common_remove(storage, NOTIFICATION_SETTINGS_OLD_PATH); // Ext -> Int FURI_LOG_I(TAG, "Migrate: External to Internal"); @@ -77,7 +77,7 @@ void flipper_migrate_files() { FileInfo file_info; if(storage_common_stat(storage, U2F_CNT_OLD_FILE, &file_info) == FSE_OK && file_info.size > 200) { // Is on Int and has content - storage_common_move(storage, U2F_CNT_OLD_FILE, U2F_CNT_FILE); // Int -> Ext + storage_common_rename(storage, U2F_CNT_OLD_FILE, U2F_CNT_FILE); // Int -> Ext } storage_common_copy(storage, U2F_KEY_OLD_FILE, U2F_KEY_FILE); // Ext -> Int diff --git a/targets/f7/api_symbols.csv b/targets/f7/api_symbols.csv index cc083b2ec..7fc97ca09 100644 --- a/targets/f7/api_symbols.csv +++ b/targets/f7/api_symbols.csv @@ -2814,9 +2814,9 @@ Function,+,storage_common_fs_info,FS_Error,"Storage*, const char*, uint64_t*, ui Function,+,storage_common_merge,FS_Error,"Storage*, const char*, const char*" Function,+,storage_common_migrate,FS_Error,"Storage*, const char*, const char*" Function,+,storage_common_mkdir,FS_Error,"Storage*, const char*" -Function,+,storage_common_move,FS_Error,"Storage*, const char*, const char*" Function,+,storage_common_remove,FS_Error,"Storage*, const char*" Function,+,storage_common_rename,FS_Error,"Storage*, const char*, const char*" +Function,+,storage_common_rename_safe,FS_Error,"Storage*, const char*, const char*" Function,+,storage_common_resolve_path_and_ensure_app_directory,void,"Storage*, FuriString*" Function,+,storage_common_stat,FS_Error,"Storage*, const char*, FileInfo*" Function,+,storage_common_timestamp,FS_Error,"Storage*, const char*, uint32_t*"