From 4140952605a60b42a8c282bc46a5ca6505eb9bf2 Mon Sep 17 00:00:00 2001 From: Anna Antonenko Date: Tue, 8 Apr 2025 18:05:08 +0400 Subject: [PATCH] cli_registry: move from bptree to dict, fix memory leak --- lib/toolbox/cli/cli_registry.c | 32 +++++++++---------- lib/toolbox/cli/cli_registry_i.h | 18 +++-------- lib/toolbox/cli/shell/cli_shell.c | 14 ++++---- lib/toolbox/cli/shell/cli_shell_completions.c | 6 ++-- 4 files changed, 29 insertions(+), 41 deletions(-) diff --git a/lib/toolbox/cli/cli_registry.c b/lib/toolbox/cli/cli_registry.c index 35bed19f2..91f7c4046 100644 --- a/lib/toolbox/cli/cli_registry.c +++ b/lib/toolbox/cli/cli_registry.c @@ -3,16 +3,16 @@ #include #include -#define TAG "cli" +#define TAG "CliRegistry" struct CliRegistry { - CliCommandTree_t commands; + CliCommandDict_t commands; FuriMutex* mutex; }; CliRegistry* cli_registry_alloc(void) { CliRegistry* registry = malloc(sizeof(CliRegistry)); - CliCommandTree_init(registry->commands); + CliCommandDict_init(registry->commands); registry->mutex = furi_mutex_alloc(FuriMutexTypeRecursive); return registry; } @@ -20,7 +20,7 @@ CliRegistry* cli_registry_alloc(void) { void cli_registry_free(CliRegistry* registry) { furi_check(furi_mutex_acquire(registry->mutex, FuriWaitForever) == FuriStatusOk); furi_mutex_free(registry->mutex); - CliCommandTree_clear(registry->commands); + CliCommandDict_clear(registry->commands); free(registry); } @@ -61,7 +61,7 @@ void cli_registry_add_command_ex( }; furi_check(furi_mutex_acquire(registry->mutex, FuriWaitForever) == FuriStatusOk); - CliCommandTree_set_at(registry->commands, name_str, command); + CliCommandDict_set_at(registry->commands, name_str, command); furi_check(furi_mutex_release(registry->mutex) == FuriStatusOk); furi_string_free(name_str); @@ -79,7 +79,7 @@ void cli_registry_delete_command(CliRegistry* registry, const char* name) { } while(name_replace != FURI_STRING_FAILURE); furi_check(furi_mutex_acquire(registry->mutex, FuriWaitForever) == FuriStatusOk); - CliCommandTree_erase(registry->commands, name_str); + CliCommandDict_erase(registry->commands, name_str); furi_check(furi_mutex_release(registry->mutex) == FuriStatusOk); furi_string_free(name_str); @@ -91,7 +91,7 @@ bool cli_registry_get_command( CliRegistryCommand* result) { furi_assert(registry); furi_check(furi_mutex_acquire(registry->mutex, FuriWaitForever) == FuriStatusOk); - CliRegistryCommand* data = CliCommandTree_get(registry->commands, command); + CliRegistryCommand* data = CliCommandDict_get(registry->commands, command); if(data) *result = *data; furi_check(furi_mutex_release(registry->mutex) == FuriStatusOk); @@ -103,16 +103,14 @@ void cli_registry_remove_external_commands(CliRegistry* registry) { furi_check(registry); furi_check(furi_mutex_acquire(registry->mutex, FuriWaitForever) == FuriStatusOk); - // FIXME FL-3977: memory leak somewhere within this function - - CliCommandTree_t internal_cmds; - CliCommandTree_init(internal_cmds); + CliCommandDict_t internal_cmds; + CliCommandDict_init(internal_cmds); for - M_EACH(item, registry->commands, CliCommandTree_t) { - if(!(item->value_ptr->flags & CliCommandFlagExternal)) - CliCommandTree_set_at(internal_cmds, *item->key_ptr, *item->value_ptr); + M_EACH(item, registry->commands, CliCommandDict_t) { + if(!(item->value.flags & CliCommandFlagExternal)) + CliCommandDict_set_at(internal_cmds, item->key, item->value); } - CliCommandTree_move(registry->commands, internal_cmds); + CliCommandDict_move(registry->commands, internal_cmds); furi_check(furi_mutex_release(registry->mutex) == FuriStatusOk); } @@ -148,7 +146,7 @@ void cli_registry_reload_external_commands( .execute_callback = NULL, .flags = CliCommandFlagExternal, }; - CliCommandTree_set_at(registry->commands, plugin_name, command); + CliCommandDict_set_at(registry->commands, plugin_name, command); } furi_string_free(plugin_name); @@ -172,7 +170,7 @@ void cli_registry_unlock(CliRegistry* registry) { furi_mutex_release(registry->mutex); } -CliCommandTree_t* cli_registry_get_commands(CliRegistry* registry) { +CliCommandDict_t* cli_registry_get_commands(CliRegistry* registry) { furi_assert(registry); return ®istry->commands; } diff --git a/lib/toolbox/cli/cli_registry_i.h b/lib/toolbox/cli/cli_registry_i.h index 95b7c55da..31995832f 100644 --- a/lib/toolbox/cli/cli_registry_i.h +++ b/lib/toolbox/cli/cli_registry_i.h @@ -6,7 +6,7 @@ #pragma once #include -#include +#include #include "cli_registry.h" #ifdef __cplusplus @@ -22,19 +22,9 @@ typedef struct { size_t stack_depth; } CliRegistryCommand; -#define CLI_COMMANDS_TREE_RANK 4 +DICT_DEF2(CliCommandDict, FuriString*, FURI_STRING_OPLIST, CliRegistryCommand, M_POD_OPLIST); -// -V:BPTREE_DEF2:1103 -// -V:BPTREE_DEF2:524 -BPTREE_DEF2( - CliCommandTree, - CLI_COMMANDS_TREE_RANK, - FuriString*, - FURI_STRING_OPLIST, - CliRegistryCommand, - M_POD_OPLIST); - -#define M_OPL_CliCommandTree_t() BPTREE_OPLIST2(CliCommandTree, FURI_STRING_OPLIST, M_POD_OPLIST) +#define M_OPL_CliCommandDict_t() DICT_OPLIST(CliCommandDict, FURI_STRING_OPLIST, M_POD_OPLIST) bool cli_registry_get_command( CliRegistry* registry, @@ -48,7 +38,7 @@ void cli_registry_unlock(CliRegistry* registry); /** * @warning Surround calls to this function with `cli_registry_[un]lock` */ -CliCommandTree_t* cli_registry_get_commands(CliRegistry* registry); +CliCommandDict_t* cli_registry_get_commands(CliRegistry* registry); #ifdef __cplusplus } diff --git a/lib/toolbox/cli/shell/cli_shell.c b/lib/toolbox/cli/shell/cli_shell.c index 445a5f2aa..8aa7c387a 100644 --- a/lib/toolbox/cli/shell/cli_shell.c +++ b/lib/toolbox/cli/shell/cli_shell.c @@ -103,15 +103,15 @@ void cli_command_help(PipeSide* pipe, FuriString* args, void* context) { printf("Available commands:\r\n" ANSI_FG_GREEN); cli_registry_lock(registry); - CliCommandTree_t* commands = cli_registry_get_commands(registry); - size_t commands_count = CliCommandTree_size(*commands); + CliCommandDict_t* commands = cli_registry_get_commands(registry); + size_t commands_count = CliCommandDict_size(*commands); - CliCommandTree_it_t iterator; - CliCommandTree_it(iterator, *commands); + CliCommandDict_it_t iterator; + CliCommandDict_it(iterator, *commands); for(size_t i = 0; i < commands_count; i++) { - const CliCommandTree_itref_t* item = CliCommandTree_cref(iterator); - printf("%-30s", furi_string_get_cstr(*item->key_ptr)); - CliCommandTree_next(iterator); + const CliCommandDict_itref_t* item = CliCommandDict_cref(iterator); + printf("%-30s", furi_string_get_cstr(item->key)); + CliCommandDict_next(iterator); if(i % columns == columns - 1) printf("\r\n"); } diff --git a/lib/toolbox/cli/shell/cli_shell_completions.c b/lib/toolbox/cli/shell/cli_shell_completions.c index 823f91fb9..7a178705d 100644 --- a/lib/toolbox/cli/shell/cli_shell_completions.c +++ b/lib/toolbox/cli/shell/cli_shell_completions.c @@ -111,10 +111,10 @@ void cli_shell_completions_fill_variants(CliShellCompletions* completions) { if(segment.type == CliShellCompletionSegmentTypeCommand) { CliRegistry* registry = completions->registry; cli_registry_lock(registry); - CliCommandTree_t* commands = cli_registry_get_commands(registry); + CliCommandDict_t* commands = cli_registry_get_commands(registry); for - M_EACH(registered_command, *commands, CliCommandTree_t) { - FuriString* command_name = *registered_command->key_ptr; + M_EACH(registered_command, *commands, CliCommandDict_t) { + FuriString* command_name = registered_command->key; if(furi_string_start_with(command_name, input)) { CommandCompletions_push_back(completions->variants, command_name); }