فهرست منبع

[FL-2502] Properly closing directory on free (#1174)

* Storage: better (at least working) mechanism to distinguish between files and dirs
* Storage API: debug log
* TarArchive: fix stream memleak
* TarArchive: fix another memleak
* Storage: better logs
* Storage: changed the log level to trace

Co-authored-by: あく <alleteam@gmail.com>
SG 3 سال پیش
والد
کامیت
c60562a02c
3فایلهای تغییر یافته به همراه33 افزوده شده و 11 حذف شده
  1. 8 0
      applications/storage/filesystem_api_internal.h
  2. 23 11
      applications/storage/storage_external_api.c
  3. 2 0
      lib/toolbox/tar/tar_archive.c

+ 8 - 0
applications/storage/filesystem_api_internal.h

@@ -6,9 +6,17 @@
 extern "C" {
 #endif
 
+/** File type */
+typedef enum {
+    FileTypeClosed, /**< Closed file */
+    FileTypeOpenDir, /**< Open dir */
+    FileTypeOpenFile, /**< Open file */
+} FileType;
+
 /** Structure that hold file index and returned api errors */
 struct File {
     uint32_t file_id; /**< File ID for internal references */
+    FileType type;
     FS_Error error_id; /**< Standart API error from FS_Error enum */
     int32_t internal_error_id; /**< Internal API error value */
     void* storage;

+ 23 - 11
applications/storage/storage_external_api.c

@@ -7,6 +7,8 @@
 
 #define MAX_NAME_LENGTH 256
 
+#define TAG "StorageAPI"
+
 #define S_API_PROLOGUE                                      \
     osSemaphoreId_t semaphore = osSemaphoreNew(1, 0, NULL); \
     furi_check(semaphore != NULL);
@@ -47,10 +49,6 @@
 #define S_RETURN_ERROR (return_data.error_value);
 #define S_RETURN_CSTRING (return_data.cstring_value);
 
-#define FILE_OPENED_FILE 1
-#define FILE_OPENED_DIR 2
-#define FILE_CLOSED 0
-
 typedef enum {
     StorageEventFlagFileClose = (1 << 0),
 } StorageEventFlag;
@@ -72,7 +70,7 @@ static bool storage_file_open_internal(
             .open_mode = open_mode,
         }};
 
-    file->file_id = FILE_OPENED_FILE;
+    file->type = FileTypeOpenFile;
 
     S_API_MESSAGE(StorageCommandFileOpen);
     S_API_EPILOGUE;
@@ -113,6 +111,10 @@ bool storage_file_open(
 
     furi_pubsub_unsubscribe(storage_get_pubsub(file->storage), subscription);
     osEventFlagsDelete(event);
+
+    FURI_LOG_T(
+        TAG, "File %p - %p open (%s)", (uint32_t)file - SRAM_BASE, file->file_id - SRAM_BASE, path);
+
     return result;
 }
 
@@ -124,7 +126,8 @@ bool storage_file_close(File* file) {
     S_API_MESSAGE(StorageCommandFileClose);
     S_API_EPILOGUE;
 
-    file->file_id = FILE_CLOSED;
+    FURI_LOG_T(TAG, "File %p - %p closed", (uint32_t)file - SRAM_BASE, file->file_id - SRAM_BASE);
+    file->type = FileTypeClosed;
 
     return S_RETURN_BOOL;
 }
@@ -234,7 +237,7 @@ static bool storage_dir_open_internal(File* file, const char* path) {
             .path = path,
         }};
 
-    file->file_id = FILE_OPENED_DIR;
+    file->type = FileTypeOpenDir;
 
     S_API_MESSAGE(StorageCommandDirOpen);
     S_API_EPILOGUE;
@@ -259,6 +262,10 @@ bool storage_dir_open(File* file, const char* path) {
 
     furi_pubsub_unsubscribe(storage_get_pubsub(file->storage), subscription);
     osEventFlagsDelete(event);
+
+    FURI_LOG_T(
+        TAG, "Dir %p - %p open (%s)", (uint32_t)file - SRAM_BASE, file->file_id - SRAM_BASE, path);
+
     return result;
 }
 
@@ -269,7 +276,9 @@ bool storage_dir_close(File* file) {
     S_API_MESSAGE(StorageCommandDirClose);
     S_API_EPILOGUE;
 
-    file->file_id = FILE_CLOSED;
+    FURI_LOG_T(TAG, "Dir %p - %p closed", (uint32_t)file - SRAM_BASE, file->file_id - SRAM_BASE);
+
+    file->type = FileTypeClosed;
 
     return S_RETURN_BOOL;
 }
@@ -448,18 +457,20 @@ FS_Error storage_sd_status(Storage* storage) {
 
 File* storage_file_alloc(Storage* storage) {
     File* file = malloc(sizeof(File));
-    file->file_id = FILE_CLOSED;
+    file->type = FileTypeClosed;
     file->storage = storage;
 
+    FURI_LOG_T(TAG, "File/Dir %p alloc", (uint32_t)file - SRAM_BASE);
+
     return file;
 }
 
 bool storage_file_is_open(File* file) {
-    return (file->file_id != FILE_CLOSED);
+    return (file->type != FileTypeClosed);
 }
 
 bool storage_file_is_dir(File* file) {
-    return (file->file_id == FILE_OPENED_DIR);
+    return (file->type == FileTypeOpenDir);
 }
 
 void storage_file_free(File* file) {
@@ -471,6 +482,7 @@ void storage_file_free(File* file) {
         }
     }
 
+    FURI_LOG_T(TAG, "File/Dir %p free", (uint32_t)file - SRAM_BASE);
     free(file);
 }
 

+ 2 - 0
lib/toolbox/tar/tar_archive.c

@@ -38,6 +38,7 @@ static int mtar_storage_file_seek(void* stream, unsigned offset) {
 static int mtar_storage_file_close(void* stream) {
     if(stream) {
         storage_file_close(stream);
+        storage_file_free(stream);
     }
     return MTAR_ESUCCESS;
 }
@@ -93,6 +94,7 @@ void tar_archive_free(TarArchive* archive) {
     if(mtar_is_open(&archive->tar)) {
         mtar_close(&archive->tar);
     }
+    free(archive);
 }
 
 void tar_archive_set_file_callback(TarArchive* archive, tar_unpack_file_cb callback, void* context) {