From f183ccbe0f6e954ec2c9d0e1a1296ac54ff637e5 Mon Sep 17 00:00:00 2001 From: Martin Kutlak Date: Tue, 25 Apr 2017 18:20:08 +0200 Subject: [PATCH 08/14] lib: replace hash table with list Order of login credentials wasn't guaranteed, because event options with missing values were inserted into hash table and then extracted from it. I renamed function validate_event as its name was a little bit confusing. The function returns options that are missing input values and error message should be displayed. This fixes abrt/abrt#1231 Signed-off-by: Martin Kutlak --- src/cli/cli-report.c | 32 ++++++------- src/gui-wizard-gtk/wizard.c | 4 +- src/include/event_config.h | 15 +++++- src/lib/event_config.c | 36 +++++++++++---- src/report-newt/report-newt.c | 25 +++++----- tests/event_config.at | 103 ++++++++++++++++++++++++++++++++++++++++++ 6 files changed, 173 insertions(+), 42 deletions(-) diff --git a/src/cli/cli-report.c b/src/cli/cli-report.c index 537b194..fd14888 100644 --- a/src/cli/cli-report.c +++ b/src/cli/cli-report.c @@ -414,19 +414,17 @@ static void ask_for_missing_settings(const char *event_name) { for (int i = 0; i < 3; ++i) { - GHashTable *error_table = validate_event(event_name); - if (!error_table) + GList *err_list = NULL, *iter = NULL; + err_list = get_options_with_err_msg(event_name); + if(!err_list) return; event_config_t *event_config = get_event_config(event_name); - GHashTableIter iter; - char *opt_name, *err_msg; - g_hash_table_iter_init(&iter, error_table); - while (g_hash_table_iter_next(&iter, (void**)&opt_name, (void**)&err_msg)) + for (iter = err_list; iter; iter = iter->next) { - event_option_t *opt = get_event_option_from_list(opt_name, - event_config->options); + invalid_option_t *err_data = (invalid_option_t *)iter->data; + event_option_t *opt = get_event_option_from_list(err_data->invopt_name, event_config->options); free(opt->eo_value); opt->eo_value = NULL; @@ -459,25 +457,27 @@ static void ask_for_missing_settings(const char *event_name) free(question); } - g_hash_table_destroy(error_table); + g_list_free_full(err_list, (GDestroyNotify)free_invalid_options); - error_table = validate_event(event_name); - if (!error_table) + err_list = get_options_with_err_msg(event_name); + if (!err_list) return; alert(_("Your input is not valid, because of:")); - g_hash_table_iter_init(&iter, error_table); - while (g_hash_table_iter_next(&iter, (void**)&opt_name, (void**)&err_msg)) + for (iter = err_list; iter; iter = iter -> next) { - char *msg = xasprintf(_("Bad value for '%s': %s"), opt_name, err_msg); + invalid_option_t *err_data = (invalid_option_t *)iter->data; + char *msg = xasprintf(_("Bad value for '%s': %s"), + err_data->invopt_name, + err_data->invopt_error); alert(msg); free(msg); } - g_hash_table_destroy(error_table); + g_list_free_full(err_list, (GDestroyNotify)free_invalid_options); } - /* we ask for 3 times and still don't have valid infromation */ + /* we ask for 3 times and still don't have valid information */ error_msg_and_die("Invalid input, exiting."); } diff --git a/src/gui-wizard-gtk/wizard.c b/src/gui-wizard-gtk/wizard.c index c7136d6..d337805 100644 --- a/src/gui-wizard-gtk/wizard.c +++ b/src/gui-wizard-gtk/wizard.c @@ -914,10 +914,10 @@ static gint find_by_button(gconstpointer a, gconstpointer button) static void check_event_config(const char *event_name) { - GHashTable *errors = validate_event(event_name); + GList *errors = get_options_with_err_msg(event_name); if (errors != NULL) { - g_hash_table_unref(errors); + g_list_free_full(errors, (GDestroyNotify)free_invalid_options); show_event_config_dialog(event_name, GTK_WINDOW(g_top_most_window)); update_private_ticket_creation_warning_for_selected_event(); } diff --git a/src/include/event_config.h b/src/include/event_config.h index fdcb3b4..0e0df98 100644 --- a/src/include/event_config.h +++ b/src/include/event_config.h @@ -63,6 +63,17 @@ typedef struct bool is_advanced; } event_option_t; +/* + * struct holds + * invopt_name = name of the option with invalid value + * invopt_error = string of the error message + */ +typedef struct +{ + char *invopt_name; + char *invopt_error; +} invalid_option_t; + event_option_t *new_event_option(void); void free_event_option(event_option_t *p); @@ -108,6 +119,8 @@ bool ec_restricted_access_enabled(event_config_t *ec); void free_event_config(event_config_t *p); +invalid_option_t *new_invalid_option(void); +void free_invalid_options(invalid_option_t* p); void load_event_description_from_file(event_config_t *event_config, const char* filename); @@ -126,7 +139,7 @@ extern GHashTable *g_event_config_list; // for iterating through entire list o GList *export_event_config(const char *event_name); void unexport_event_config(GList *env_list); -GHashTable *validate_event(const char *event_name); +GList *get_options_with_err_msg(const char *event_name); /* * Checks usability of problem's backtrace rating against required rating level diff --git a/src/lib/event_config.c b/src/lib/event_config.c index 9c58d00..c5b7b17 100644 --- a/src/lib/event_config.c +++ b/src/lib/event_config.c @@ -22,6 +22,11 @@ GHashTable *g_event_config_list; static GHashTable *g_event_config_symlinks; +invalid_option_t *new_invalid_option(void) +{ + return xzalloc(sizeof(invalid_option_t)); +} + event_option_t *new_event_option(void) { return xzalloc(sizeof(event_option_t)); @@ -121,6 +126,15 @@ bool ec_restricted_access_enabled(event_config_t *ec) return eo->eo_value != NULL && string_to_bool(eo->eo_value); } +void free_invalid_options(invalid_option_t *p) +{ + if (!p) + return; + free(p->invopt_name); + free(p->invopt_error); + free(p); +} + void free_event_option(event_option_t *p) { if (!p) @@ -427,7 +441,7 @@ static char *validate_event_option(event_option_t *opt) return NULL; } -GHashTable *validate_event(const char *event_name) +GList *get_options_with_err_msg(const char *event_name) { INITIALIZE_LIBREPORT(); @@ -435,21 +449,23 @@ GHashTable *validate_event(const char *event_name) if (!config) return NULL; - GHashTable *errors = g_hash_table_new_full(g_str_hash, g_str_equal, free, free); - GList *li; + GList *iter, *err_list = NULL; - for (li = config->options; li; li = li->next) + for (iter = config->options; iter; iter = iter->next) { - event_option_t *opt = (event_option_t *)li->data; + event_option_t *opt = (event_option_t *)iter->data; char *err = validate_event_option(opt); if (err) - g_hash_table_insert(errors, xstrdup(opt->eo_name), err); + { + invalid_option_t *inv_opt = new_invalid_option(); + inv_opt->invopt_name = xstrdup(opt->eo_name); + inv_opt->invopt_error = xstrdup(err); + err_list = g_list_prepend(err_list, inv_opt); + } } - if (g_hash_table_size(errors)) - return errors; - - g_hash_table_destroy(errors); + if (err_list != NULL) + return g_list_reverse(err_list); return NULL; } diff --git a/src/report-newt/report-newt.c b/src/report-newt/report-newt.c index 2427d86..61e279d 100644 --- a/src/report-newt/report-newt.c +++ b/src/report-newt/report-newt.c @@ -93,15 +93,14 @@ static int select_reporters(GArray *reporters) static int configure_reporter(struct reporter *r, bool skip_if_valid) { - GHashTable *error_table; - GList *option; + GList *error_list, *option; event_option_t *opt; bool first = true, cancel = false; int num_opts, i; newtComponent text, *options, button_ok, button_cancel, form; newtGrid grid, ogrid, bgrid; - while ((error_table = validate_event(r->name)) || + while ((error_list = get_options_with_err_msg(r->name)) || (!skip_if_valid && first && r->config)) { text = newtTextboxReflowed(0, 0, ec_get_screen_name(r->config) ? @@ -151,19 +150,19 @@ static int configure_reporter(struct reporter *r, bool skip_if_valid) form = newtForm(NULL, NULL, 0); newtGridAddComponentsToForm(grid, form, 1); - if (!first && error_table) + if (!first && error_list) { - GHashTableIter iter; - char *opt_name, *err_msg, buf[4096]; + GList *iter; + char buf[4096]; /* Catenate the error messages */ buf[0] = '\0'; - for (g_hash_table_iter_init(&iter, error_table); - g_hash_table_iter_next(&iter, (void**)&opt_name, (void**)&err_msg); ) + for (iter = error_list; iter; iter = iter->next) { - opt = get_event_option_from_list(opt_name, r->config->options); + invalid_option_t *inv_data = (invalid_option_t *)iter->data; + opt = get_event_option_from_list(inv_data->invopt_name, r->config->options); snprintf(buf + strlen(buf), sizeof (buf) - strlen(buf), "%s: %s\n", - opt->eo_label ? opt->eo_label : opt->eo_name, err_msg); + opt->eo_label ? opt->eo_label : opt->eo_name, inv_data->invopt_error); } newtWinMessage(_("Error"), _("Ok"), buf); @@ -201,14 +200,14 @@ static int configure_reporter(struct reporter *r, bool skip_if_valid) free(options); - if (error_table) - g_hash_table_destroy(error_table); + if (error_list) + g_list_free_full(error_list,(GDestroyNotify)free_invalid_options); if (cancel) break; first = false; } - return !error_table; + return !error_list; } struct log { diff --git a/tests/event_config.at b/tests/event_config.at index 5baf000..badca73 100644 --- a/tests/event_config.at +++ b/tests/event_config.at @@ -58,3 +58,106 @@ TS_MAIN } TS_RETURN_MAIN ]]) + +## ------------------------ ## +## get_options_with_err_msg ## +## ------------------------ ## + +AT_TESTFUN([get_options_with_err_msg], [[ +#include "testsuite.h" +#include "internal_libreport.h" + +event_option_t* create_new_option(const char *n, const char *v, option_type_t t, int ae) +{ + event_option_t *op = new_event_option(); + op->eo_name = xstrdup(n); + op->eo_value = NULL; + if(v != NULL) + op->eo_value = xstrdup(v); + + op->eo_type = t; + op->eo_allow_empty = ae; + + return op; +} + +TS_MAIN +{ + GList *errors = NULL, *iter = NULL; + invalid_option_t *e_op; + + if (!g_event_config_list) + g_event_config_list = g_hash_table_new_full( + g_str_hash, g_str_equal, free, (GDestroyNotify) free_event_config + ); + + { + event_config_t *evnt = new_event_config("Bugster0"); + event_option_t *opt_login = create_new_option("Bugtest_Login", NULL, OPTION_TYPE_TEXT, 0); + event_option_t *opt_passwd = create_new_option("Bugtest_Password", NULL, OPTION_TYPE_PASSWORD, 0); + event_option_t *opt_url = create_new_option("Bugtest_URL", "bug.test", OPTION_TYPE_TEXT, 0); + + evnt->options = g_list_append(evnt->options, opt_login); + evnt->options = g_list_append(evnt->options, opt_passwd); + evnt->options = g_list_append(evnt->options, opt_url); + g_hash_table_insert(g_event_config_list, xstrdup("Bugster0"), evnt); + + errors = get_options_with_err_msg("Bugster0"); + e_op = (invalid_option_t *)errors->data; + + TS_ASSERT_STRING_EQ(e_op->invopt_name, "Bugtest_Login", "Show login first"); + + iter = g_list_next(errors); + e_op = (invalid_option_t *)iter->data; + + TS_ASSERT_STRING_EQ(e_op->invopt_name, "Bugtest_Password", "Show password second"); + TS_ASSERT_PTR_IS_NULL(g_list_next(iter)); + + g_list_free_full(errors, (GDestroyNotify)free_invalid_options); + } + + { + event_config_t *evnt = new_event_config("Bugster1"); + event_option_t *opt_login = create_new_option("Bugtest_Login", NULL, OPTION_TYPE_TEXT, 0); + event_option_t *opt_passwd = create_new_option("Bugtest_Password", NULL, OPTION_TYPE_PASSWORD, 0); + event_option_t *opt_url = create_new_option("Bugtest_URL", "bug.test", OPTION_TYPE_TEXT, 0); + + evnt->options = g_list_append(evnt->options, opt_passwd); + evnt->options = g_list_append(evnt->options, opt_login); + evnt->options = g_list_append(evnt->options, opt_url); + g_hash_table_insert(g_event_config_list, xstrdup("Bugster1"), evnt); + + errors = get_options_with_err_msg("Bugster1"); + e_op = (invalid_option_t *)errors->data; + + TS_ASSERT_STRING_EQ(e_op->invopt_name, "Bugtest_Password", "Show password first"); + + iter = g_list_next(errors); + e_op = (invalid_option_t *)iter->data; + + TS_ASSERT_STRING_EQ(e_op->invopt_name, "Bugtest_Login", "Show login second"); + TS_ASSERT_PTR_IS_NULL(g_list_next(iter)); + + g_list_free_full(errors, (GDestroyNotify)free_invalid_options); + } + + { + event_config_t *evnt = new_event_config("Bugster2"); + event_option_t *opt_login = create_new_option("Bugtest_Login", "login", OPTION_TYPE_TEXT, 0); + event_option_t *opt_passwd = create_new_option("Bugtest_Password", "password", OPTION_TYPE_PASSWORD, 0); + event_option_t *opt_url = create_new_option("Bugtest_URL", "bug.test", OPTION_TYPE_TEXT, 0); + + evnt->options = g_list_append(evnt->options, opt_login); + evnt->options = g_list_append(evnt->options, opt_passwd); + evnt->options = g_list_append(evnt->options, opt_url); + g_hash_table_insert(g_event_config_list, xstrdup("Bugster2"), evnt); + + errors = get_options_with_err_msg("Bugster2"); + + TS_ASSERT_PTR_IS_NULL(errors); + } + + free_event_config_data(); +} +TS_RETURN_MAIN +]]) -- 2.9.5