Przeglądaj źródła

[FL-2423] Storage: blocking dir open (#1083)

* Storage: more unit tests
* Storage: blocking dir open, differentiate file and dir when freed.

Co-authored-by: あく <alleteam@gmail.com>
SG 3 lat temu
rodzic
commit
e5a1f20fd4

+ 7 - 0
applications/storage/storage.h

@@ -24,6 +24,7 @@ typedef enum {
     StorageEventTypeCardUnmount,
     StorageEventTypeCardUnmount,
     StorageEventTypeCardMountError,
     StorageEventTypeCardMountError,
     StorageEventTypeFileClose,
     StorageEventTypeFileClose,
+    StorageEventTypeDirClose,
 } StorageEventType;
 } StorageEventType;
 
 
 typedef struct {
 typedef struct {
@@ -65,6 +66,12 @@ bool storage_file_close(File* file);
  */
  */
 bool storage_file_is_open(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
 /** Reads bytes from a file into a buffer
  * @param file pointer to file object.
  * @param file pointer to file object.
  * @param buff pointer to a buffer, for reading
  * @param buff pointer to a buffer, for reading

+ 37 - 6
applications/storage/storage_external_api.c

@@ -47,7 +47,8 @@
 #define S_RETURN_ERROR (return_data.error_value);
 #define S_RETURN_ERROR (return_data.error_value);
 #define S_RETURN_CSTRING (return_data.cstring_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
 #define FILE_CLOSED 0
 
 
 typedef enum {
 typedef enum {
@@ -71,7 +72,7 @@ static bool storage_file_open_internal(
             .open_mode = open_mode,
             .open_mode = open_mode,
         }};
         }};
 
 
-    file->file_id = FILE_OPENED;
+    file->file_id = FILE_OPENED_FILE;
 
 
     S_API_MESSAGE(StorageCommandFileOpen);
     S_API_MESSAGE(StorageCommandFileOpen);
     S_API_EPILOGUE;
     S_API_EPILOGUE;
@@ -82,7 +83,8 @@ static bool storage_file_open_internal(
 static void storage_file_close_callback(const void* message, void* context) {
 static void storage_file_close_callback(const void* message, void* context) {
     const StorageEvent* storage_event = message;
     const StorageEvent* storage_event = message;
 
 
-    if(storage_event->type == StorageEventTypeFileClose) {
+    if(storage_event->type == StorageEventTypeFileClose ||
+       storage_event->type == StorageEventTypeDirClose) {
         furi_assert(context);
         furi_assert(context);
         osEventFlagsId_t event = context;
         osEventFlagsId_t event = context;
         osEventFlagsSet(event, StorageEventFlagFileClose);
         osEventFlagsSet(event, StorageEventFlagFileClose);
@@ -222,7 +224,7 @@ bool storage_file_eof(File* file) {
 
 
 /****************** DIR ******************/
 /****************** 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_FILE_API_PROLOGUE;
     S_API_PROLOGUE;
     S_API_PROLOGUE;
 
 
@@ -232,13 +234,34 @@ bool storage_dir_open(File* file, const char* path) {
             .path = path,
             .path = path,
         }};
         }};
 
 
-    file->file_id = FILE_OPENED;
+    file->file_id = FILE_OPENED_DIR;
 
 
     S_API_MESSAGE(StorageCommandDirOpen);
     S_API_MESSAGE(StorageCommandDirOpen);
     S_API_EPILOGUE;
     S_API_EPILOGUE;
     return S_RETURN_BOOL;
     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) {
 bool storage_dir_close(File* file) {
     S_FILE_API_PROLOGUE;
     S_FILE_API_PROLOGUE;
     S_API_PROLOGUE;
     S_API_PROLOGUE;
@@ -435,9 +458,17 @@ bool storage_file_is_open(File* file) {
     return (file->file_id != FILE_CLOSED);
     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) {
 void storage_file_free(File* file) {
     if(storage_file_is_open(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);
     free(file);

+ 3 - 0
applications/storage/storage_processing.c

@@ -285,6 +285,9 @@ bool storage_process_dir_close(Storage* app, File* file) {
     } else {
     } else {
         FS_CALL(storage, dir.close(storage, file));
         FS_CALL(storage, dir.close(storage, file));
         storage_pop_storage_file(file, storage);
         storage_pop_storage_file(file, storage);
+
+        StorageEvent event = {.type = StorageEventTypeDirClose};
+        furi_pubsub_publish(app->pubsub, &event);
     }
     }
 
 
     return ret;
     return ret;

+ 91 - 0
applications/tests/storage/storage_test.c

@@ -4,6 +4,7 @@
 #include <storage/storage.h>
 #include <storage/storage.h>
 
 
 #define STORAGE_LOCKED_FILE "/ext/locked_file.test"
 #define STORAGE_LOCKED_FILE "/ext/locked_file.test"
+#define STORAGE_LOCKED_DIR "/int"
 
 
 static void storage_file_open_lock_setup() {
 static void storage_file_open_lock_setup() {
     Storage* storage = furi_record_open("storage");
     Storage* storage = furi_record_open("storage");
@@ -68,13 +69,103 @@ MU_TEST(storage_file_open_lock) {
     mu_assert(result, "cannot open locked file");
     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) {
 MU_TEST_SUITE(storage_file) {
     storage_file_open_lock_setup();
     storage_file_open_lock_setup();
+    MU_RUN_TEST(storage_file_open_close);
     MU_RUN_TEST(storage_file_open_lock);
     MU_RUN_TEST(storage_file_open_lock);
     storage_file_open_lock_teardown();
     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() {
 int run_minunit_test_storage() {
     MU_RUN_SUITE(storage_file);
     MU_RUN_SUITE(storage_file);
+    MU_RUN_SUITE(storage_dir);
     return MU_EXIT_CODE;
     return MU_EXIT_CODE;
 }
 }