From e5a1f20fd4e1a9ac86160ed776d9a68c72011975 Mon Sep 17 00:00:00 2001 From: SG Date: Mon, 4 Apr 2022 22:27:48 +1000 Subject: [PATCH] [FL-2423] Storage: blocking dir open (#1083) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * Storage: more unit tests * Storage: blocking dir open, differentiate file and dir when freed. Co-authored-by: あく --- applications/storage/storage.h | 7 ++ applications/storage/storage_external_api.c | 43 ++++++++-- applications/storage/storage_processing.c | 3 + applications/tests/storage/storage_test.c | 91 +++++++++++++++++++++ 4 files changed, 138 insertions(+), 6 deletions(-) diff --git a/applications/storage/storage.h b/applications/storage/storage.h index cf18d4853..1ff832e62 100644 --- a/applications/storage/storage.h +++ b/applications/storage/storage.h @@ -24,6 +24,7 @@ typedef enum { StorageEventTypeCardUnmount, StorageEventTypeCardMountError, StorageEventTypeFileClose, + StorageEventTypeDirClose, } StorageEventType; typedef struct { @@ -65,6 +66,12 @@ bool storage_file_close(File* file); */ bool storage_file_is_open(File* file); +/** Tells if the file is a directory + * @param file pointer to a file object + * @return bool true if file is a directory + */ +bool storage_file_is_dir(File* file); + /** Reads bytes from a file into a buffer * @param file pointer to file object. * @param buff pointer to a buffer, for reading diff --git a/applications/storage/storage_external_api.c b/applications/storage/storage_external_api.c index 927bc80ee..c6aa27145 100644 --- a/applications/storage/storage_external_api.c +++ b/applications/storage/storage_external_api.c @@ -47,7 +47,8 @@ #define S_RETURN_ERROR (return_data.error_value); #define S_RETURN_CSTRING (return_data.cstring_value); -#define FILE_OPENED 1 +#define FILE_OPENED_FILE 1 +#define FILE_OPENED_DIR 2 #define FILE_CLOSED 0 typedef enum { @@ -71,7 +72,7 @@ static bool storage_file_open_internal( .open_mode = open_mode, }}; - file->file_id = FILE_OPENED; + file->file_id = FILE_OPENED_FILE; S_API_MESSAGE(StorageCommandFileOpen); S_API_EPILOGUE; @@ -82,7 +83,8 @@ static bool storage_file_open_internal( static void storage_file_close_callback(const void* message, void* context) { const StorageEvent* storage_event = message; - if(storage_event->type == StorageEventTypeFileClose) { + if(storage_event->type == StorageEventTypeFileClose || + storage_event->type == StorageEventTypeDirClose) { furi_assert(context); osEventFlagsId_t event = context; osEventFlagsSet(event, StorageEventFlagFileClose); @@ -222,7 +224,7 @@ bool storage_file_eof(File* file) { /****************** DIR ******************/ -bool storage_dir_open(File* file, const char* path) { +static bool storage_dir_open_internal(File* file, const char* path) { S_FILE_API_PROLOGUE; S_API_PROLOGUE; @@ -232,13 +234,34 @@ bool storage_dir_open(File* file, const char* path) { .path = path, }}; - file->file_id = FILE_OPENED; + file->file_id = FILE_OPENED_DIR; S_API_MESSAGE(StorageCommandDirOpen); S_API_EPILOGUE; return S_RETURN_BOOL; } +bool storage_dir_open(File* file, const char* path) { + bool result; + osEventFlagsId_t event = osEventFlagsNew(NULL); + FuriPubSubSubscription* subscription = furi_pubsub_subscribe( + storage_get_pubsub(file->storage), storage_file_close_callback, event); + + do { + result = storage_dir_open_internal(file, path); + + if(!result && file->error_id == FSE_ALREADY_OPEN) { + osEventFlagsWait(event, StorageEventFlagFileClose, osFlagsWaitAny, osWaitForever); + } else { + break; + } + } while(true); + + furi_pubsub_unsubscribe(storage_get_pubsub(file->storage), subscription); + osEventFlagsDelete(event); + return result; +} + bool storage_dir_close(File* file) { S_FILE_API_PROLOGUE; S_API_PROLOGUE; @@ -435,9 +458,17 @@ bool storage_file_is_open(File* file) { return (file->file_id != FILE_CLOSED); } +bool storage_file_is_dir(File* file) { + return (file->file_id == FILE_OPENED_DIR); +} + void storage_file_free(File* file) { if(storage_file_is_open(file)) { - storage_file_close(file); + if(storage_file_is_dir(file)) { + storage_dir_close(file); + } else { + storage_file_close(file); + } } free(file); diff --git a/applications/storage/storage_processing.c b/applications/storage/storage_processing.c index a96d9cfc5..bb1ac9c30 100644 --- a/applications/storage/storage_processing.c +++ b/applications/storage/storage_processing.c @@ -285,6 +285,9 @@ bool storage_process_dir_close(Storage* app, File* file) { } else { FS_CALL(storage, dir.close(storage, file)); storage_pop_storage_file(file, storage); + + StorageEvent event = {.type = StorageEventTypeDirClose}; + furi_pubsub_publish(app->pubsub, &event); } return ret; diff --git a/applications/tests/storage/storage_test.c b/applications/tests/storage/storage_test.c index e6ae8c812..3fccc68f2 100644 --- a/applications/tests/storage/storage_test.c +++ b/applications/tests/storage/storage_test.c @@ -4,6 +4,7 @@ #include #define STORAGE_LOCKED_FILE "/ext/locked_file.test" +#define STORAGE_LOCKED_DIR "/int" static void storage_file_open_lock_setup() { Storage* storage = furi_record_open("storage"); @@ -68,13 +69,103 @@ MU_TEST(storage_file_open_lock) { mu_assert(result, "cannot open locked file"); } +MU_TEST(storage_file_open_close) { + Storage* storage = furi_record_open("storage"); + File* file; + + file = storage_file_alloc(storage); + mu_check(storage_file_open(file, STORAGE_LOCKED_FILE, FSAM_READ_WRITE, FSOM_OPEN_EXISTING)); + storage_file_close(file); + storage_file_free(file); + + for(size_t i = 0; i < 10; i++) { + file = storage_file_alloc(storage); + mu_check( + storage_file_open(file, STORAGE_LOCKED_FILE, FSAM_READ_WRITE, FSOM_OPEN_EXISTING)); + storage_file_free(file); + } + + furi_record_close("storage"); +} + MU_TEST_SUITE(storage_file) { storage_file_open_lock_setup(); + MU_RUN_TEST(storage_file_open_close); MU_RUN_TEST(storage_file_open_lock); storage_file_open_lock_teardown(); } +MU_TEST(storage_dir_open_close) { + Storage* storage = furi_record_open("storage"); + File* file; + + file = storage_file_alloc(storage); + mu_check(storage_dir_open(file, STORAGE_LOCKED_DIR)); + storage_dir_close(file); + storage_file_free(file); + + for(size_t i = 0; i < 10; i++) { + file = storage_file_alloc(storage); + mu_check(storage_dir_open(file, STORAGE_LOCKED_DIR)); + storage_file_free(file); + } + + furi_record_close("storage"); +} + +static int32_t storage_dir_locker(void* ctx) { + Storage* storage = furi_record_open("storage"); + osSemaphoreId_t semaphore = ctx; + File* file = storage_file_alloc(storage); + furi_check(storage_dir_open(file, STORAGE_LOCKED_DIR)); + osSemaphoreRelease(semaphore); + furi_hal_delay_ms(1000); + + furi_check(storage_dir_close(file)); + furi_record_close("storage"); + storage_file_free(file); + return 0; +} + +MU_TEST(storage_dir_open_lock) { + Storage* storage = furi_record_open("storage"); + bool result = false; + osSemaphoreId_t semaphore = osSemaphoreNew(1, 0, NULL); + File* file = storage_file_alloc(storage); + + // file_locker thread start + FuriThread* locker_thread = furi_thread_alloc(); + furi_thread_set_name(locker_thread, "StorageDirLocker"); + furi_thread_set_stack_size(locker_thread, 2048); + furi_thread_set_context(locker_thread, semaphore); + furi_thread_set_callback(locker_thread, storage_dir_locker); + mu_check(furi_thread_start(locker_thread)); + + // wait for dir lock + osSemaphoreAcquire(semaphore, osWaitForever); + osSemaphoreDelete(semaphore); + + result = storage_dir_open(file, STORAGE_LOCKED_DIR); + storage_dir_close(file); + + // file_locker thread stop + mu_check(furi_thread_join(locker_thread) == osOK); + furi_thread_free(locker_thread); + + // clean data + storage_file_free(file); + furi_record_close("storage"); + + mu_assert(result, "cannot open locked dir"); +} + +MU_TEST_SUITE(storage_dir) { + MU_RUN_TEST(storage_dir_open_close); + MU_RUN_TEST(storage_dir_open_lock); +} + int run_minunit_test_storage() { MU_RUN_SUITE(storage_file); + MU_RUN_SUITE(storage_dir); return MU_EXIT_CODE; } \ No newline at end of file