From f183ccbe0f6e954ec2c9d0e1a1296ac54ff637e5 Mon Sep 17 00:00:00 2001
From: Martin Kutlak <mkutlak@redhat.com>
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 <mkutlak@redhat.com>
---
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