From 3ef4f9fd1e6ae233adb0bbfd90573623ee1059e6 Mon Sep 17 00:00:00 2001 From: Vito Caputo Date: Tue, 4 Jul 2023 20:50:56 -0700 Subject: modules/*: return invalid setting on -EINVAL in setup In the interests of improving error handling of interactive setups, return the setting that was invalid when setup returns -EINVAL. For now this is only supported for non-finalized/non-baking setup calls, i.e. (!res_setup). In the future I want this also supported for finalizing when res_setup is set. A lot of the -EINVAL returns are actually during that stage (or need to be added) due to that being when the more sensitive parsing occurs going from strings to native numeric types and such. The main reason it's not already being done in this commit is it'll churn quite a bit to get there, since the setup funcs don't generally have the setting pointers onhand at that phase. It'll require changing the string-value-centric local variables to instead all be the til_setting_t* holding those values. And if setup funcs aren't going to be so value-centric, the settings API will likely change to not even return the values directly anymore and only return the full-blown til_settings_t as the main return pointer, perhaps rid of their res_setting use even. Anyway, while here I did some random little cleanups too. --- src/modules/compose/compose.c | 29 +++++++++++++++++------------ 1 file changed, 17 insertions(+), 12 deletions(-) (limited to 'src/modules/compose') diff --git a/src/modules/compose/compose.c b/src/modules/compose/compose.c index cf1c9b1..5a10113 100644 --- a/src/modules/compose/compose.c +++ b/src/modules/compose/compose.c @@ -314,13 +314,10 @@ static int compose_setup(const til_settings_t *settings, til_setting_t **res_set */ for (size_t i = 0; til_settings_get_value_by_idx(layers_settings, i, &layer_setting); i++) { til_setting_t *layer_module_setting; - const char *layer = til_settings_get_value_by_idx(layer_setting->value_as_nested_settings, 0, &layer_module_setting); - const til_module_t *layer_module = til_lookup_module(layer); - - if (!layer_module || !layer_module_setting) - return -EINVAL; + const char *layer_module_name = til_settings_get_value_by_idx(layer_setting->value_as_nested_settings, 0, &layer_module_setting); + const til_module_t *layer_module; - if (!layer_module_setting->desc) { + if (!layer_module_name || !layer_module_setting->desc) { r = til_setting_desc_new( layer_setting->value_as_nested_settings, &(til_setting_spec_t){ .name = "Layer module name", @@ -330,11 +327,18 @@ static int compose_setup(const til_settings_t *settings, til_setting_t **res_set if (r < 0) return r; - *res_setting = layer_module_setting; + *res_setting = layer_module_name ? layer_module_setting : NULL; return 1; } + layer_module = til_lookup_module(layer_module_name); + if (!layer_module) { + *res_setting = layer_module_setting; + + return -EINVAL; + } + if (layer_module->setup) { r = layer_module->setup(layer_setting->value_as_nested_settings, res_setting, res_desc, NULL); if (r) @@ -364,10 +368,8 @@ static int compose_setup(const til_settings_t *settings, til_setting_t **res_set assert(res_setting && *res_setting && (*res_setting)->value_as_nested_settings); texture_settings = (*res_setting)->value_as_nested_settings; texture = til_settings_get_value_by_idx(texture_settings, 0, &texture_module_setting); - if (!texture) - return -EINVAL; - if (!texture_module_setting->desc) { + if (!texture || !texture_module_setting->desc) { r = til_setting_desc_new(texture_settings, &(til_setting_spec_t){ /* this is basically just to get the .as_label */ @@ -379,7 +381,7 @@ static int compose_setup(const til_settings_t *settings, til_setting_t **res_set if (r < 0) return r; - *res_setting = texture_module_setting; + *res_setting = texture ? texture_module_setting : NULL; return 1; } @@ -387,8 +389,11 @@ static int compose_setup(const til_settings_t *settings, til_setting_t **res_set if (strcasecmp(texture, "none")) { const til_module_t *texture_module = til_lookup_module(texture); - if (!texture_module) + if (!texture_module) { + *res_setting = texture_module_setting; + return -EINVAL; + } if (texture_module->setup) { r = texture_module->setup(texture_settings, res_setting, res_desc, NULL); -- cgit v1.2.3