From 75354ec5bac8be5f631213c8e236123c2bcea075 Mon Sep 17 00:00:00 2001 From: Petr Portnov | PROgrm_JARvis Date: Tue, 27 Jun 2023 12:46:04 +0300 Subject: [PATCH] fix: make `dialog_file_browser_set_basic_options` initialize all fields (#2756) * fix: make `dialog_file_browser_set_basic_options` initialize all fields * fix(GH-2756): use alternative test for `test_dialog_file_browser_set_basic_options_should_init_all_fields` Co-authored-by: Aleksandr Kutuzov --- CODING_STYLE.md | 2 +- .../dialogs/dialogs_file_browser_options.c | 32 +++++++++++++++++++ applications/debug/unit_tests/test_index.c | 3 ++ applications/services/dialogs/dialogs.c | 3 +- applications/services/dialogs/dialogs.h | 9 ++++-- 5 files changed, 44 insertions(+), 5 deletions(-) create mode 100644 applications/debug/unit_tests/dialogs/dialogs_file_browser_options.c diff --git a/CODING_STYLE.md b/CODING_STYLE.md index c62009eff..002c67f24 100644 --- a/CODING_STYLE.md +++ b/CODING_STYLE.md @@ -48,7 +48,7 @@ Almost everything in flipper firmware is built around this concept. # C coding style - Tab is 4 spaces -- Use `fbt format` to reformat source code and check style guide before commit +- Use `./fbt format` to reformat source code and check style guide before commit ## Naming diff --git a/applications/debug/unit_tests/dialogs/dialogs_file_browser_options.c b/applications/debug/unit_tests/dialogs/dialogs_file_browser_options.c new file mode 100644 index 000000000..2d5bad4c8 --- /dev/null +++ b/applications/debug/unit_tests/dialogs/dialogs_file_browser_options.c @@ -0,0 +1,32 @@ +#include + +#include "../minunit.h" + +MU_TEST(test_dialog_file_browser_set_basic_options_should_init_all_fields) { + mu_assert( + sizeof(DialogsFileBrowserOptions) == 28, + "Changes to `DialogsFileBrowserOptions` should also be reflected in `dialog_file_browser_set_basic_options`"); + + DialogsFileBrowserOptions options; + dialog_file_browser_set_basic_options(&options, ".fap", NULL); + // note: this assertions can safely be changed, their primary purpose is to remind the maintainer + // to update `dialog_file_browser_set_basic_options` by including all structure fields in it + mu_assert_string_eq(".fap", options.extension); + mu_assert_null(options.base_path); + mu_assert(options.skip_assets, "`skip_assets` should default to `true"); + mu_assert(options.hide_dot_files, "`hide_dot_files` should default to `true"); + mu_assert_null(options.icon); + mu_assert(options.hide_ext, "`hide_ext` should default to `true"); + mu_assert_null(options.item_loader_callback); + mu_assert_null(options.item_loader_context); +} + +MU_TEST_SUITE(dialogs_file_browser_options) { + MU_RUN_TEST(test_dialog_file_browser_set_basic_options_should_init_all_fields); +} + +int run_minunit_test_dialogs_file_browser_options() { + MU_RUN_SUITE(dialogs_file_browser_options); + + return MU_EXIT_CODE; +} diff --git a/applications/debug/unit_tests/test_index.c b/applications/debug/unit_tests/test_index.c index ac71ca397..9d7631bfe 100644 --- a/applications/debug/unit_tests/test_index.c +++ b/applications/debug/unit_tests/test_index.c @@ -27,6 +27,7 @@ int run_minunit_test_nfc(); int run_minunit_test_bit_lib(); int run_minunit_test_float_tools(); int run_minunit_test_bt(); +int run_minunit_test_dialogs_file_browser_options(); typedef int (*UnitTestEntry)(); @@ -55,6 +56,8 @@ const UnitTest unit_tests[] = { {.name = "bit_lib", .entry = run_minunit_test_bit_lib}, {.name = "float_tools", .entry = run_minunit_test_float_tools}, {.name = "bt", .entry = run_minunit_test_bt}, + {.name = "dialogs_file_browser_options", + .entry = run_minunit_test_dialogs_file_browser_options}, }; void minunit_print_progress() { diff --git a/applications/services/dialogs/dialogs.c b/applications/services/dialogs/dialogs.c index 3908ca31b..10c08a991 100644 --- a/applications/services/dialogs/dialogs.c +++ b/applications/services/dialogs/dialogs.c @@ -9,12 +9,13 @@ void dialog_file_browser_set_basic_options( const char* extension, const Icon* icon) { options->extension = extension; + options->base_path = NULL; options->skip_assets = true; + options->hide_dot_files = true; options->icon = icon; options->hide_ext = true; options->item_loader_callback = NULL; options->item_loader_context = NULL; - options->base_path = NULL; } static DialogsApp* dialogs_app_alloc() { diff --git a/applications/services/dialogs/dialogs.h b/applications/services/dialogs/dialogs.h index 4c1b675a6..39b15c67c 100644 --- a/applications/services/dialogs/dialogs.h +++ b/applications/services/dialogs/dialogs.h @@ -16,7 +16,8 @@ typedef struct DialogsApp DialogsApp; /****************** FILE BROWSER ******************/ /** - * File browser dialog extra options + * File browser dialog extra options. + * This can be default-initialized using {@link dialog_file_browser_set_basic_options}. * @param extension file extension to be offered for selection * @param base_path root folder path for navigation with back key * @param skip_assets true - do not show assets folders @@ -38,8 +39,10 @@ typedef struct { } DialogsFileBrowserOptions; /** - * Initialize file browser dialog options - * and set default values + * Initialize file browser dialog options and set default values. + * This is guaranteed to initialize all fields + * so it is safe to pass pointer to uninitialized {@code options} + * and assume that the data behind it becomes fully initialized after the call. * @param options pointer to options structure * @param extension file extension to filter * @param icon file icon pointer, NULL for default icon