From cc68fc5e9a7346d501c8c7df8444acc1f538ef58 Mon Sep 17 00:00:00 2001 From: James Booth Date: Mon, 14 Apr 2014 20:32:51 +0100 Subject: [PATCH 1/6] Show console message on room autojoin --- src/ui/core.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/ui/core.c b/src/ui/core.c index 9c7d321f..e96d5fff 100644 --- a/src/ui/core.c +++ b/src/ui/core.c @@ -1235,6 +1235,9 @@ _ui_room_join(char *room, gboolean focus) ui_switch_win(num); } else { status_bar_active(num); + ProfWin *console = wins_get_console(); + win_vprint_line(console, '!', COLOUR_ONLINE, "-> Autojoined %s (%d).", room, num); + win_update_virtual(console); } } From 8d77930eceba6841673605f7df5083657a0ecd28 Mon Sep 17 00:00:00 2001 From: James Booth Date: Mon, 14 Apr 2014 22:48:18 +0100 Subject: [PATCH 2/6] Added parse_options --- src/tools/parser.c | 40 +++++++++++ src/tools/parser.h | 4 +- tests/test_parser.c | 165 ++++++++++++++++++++++++++++++++++++++++++++ tests/test_parser.h | 8 +++ tests/testsuite.c | 8 +++ 5 files changed, 224 insertions(+), 1 deletion(-) diff --git a/src/tools/parser.c b/src/tools/parser.c index 932fd9aa..375572e0 100644 --- a/src/tools/parser.c +++ b/src/tools/parser.c @@ -372,3 +372,43 @@ get_start(char *string, int tokens) return result_str; } + +GHashTable * +parse_options(gchar **args, int start, GList *keys, gboolean *res) +{ + GHashTable *options = NULL; + + // no options found, success + if (args[start] == NULL) { + options = g_hash_table_new(g_str_hash, g_str_equal); + *res = TRUE; + return options; + } + + // check each option is valid and has value, failure if not + int curr; + for (curr = start; curr < g_strv_length(args); curr+= 2) { + if ((g_list_find(keys, args[curr]) == NULL) || (args[curr+1] == NULL)) { + *res = FALSE; + return options; + } + } + + // create map + options = g_hash_table_new(g_str_hash, g_str_equal); + *res = TRUE; + for (curr = start; curr < g_strv_length(args); curr+=2) { + g_hash_table_insert(options, args[curr], args[curr+1]); + } + + return options; +} + +void +options_destroy(GHashTable *options) +{ + if (options != NULL) { + g_hash_table_destroy(options); + options = NULL; + } +} \ No newline at end of file diff --git a/src/tools/parser.h b/src/tools/parser.h index f03fca81..aef3ad37 100644 --- a/src/tools/parser.h +++ b/src/tools/parser.h @@ -29,5 +29,7 @@ gchar** parse_args(const char * const inp, int min, int max, gboolean *result); gchar** parse_args_with_freetext(const char * const inp, int min, int max, gboolean *result); int count_tokens(char *string); char* get_start(char *string, int tokens); +GHashTable* parse_options(gchar **args, int start, GList *keys, gboolean *res); +void options_destroy(GHashTable *options); -#endif +#endif \ No newline at end of file diff --git a/tests/test_parser.c b/tests/test_parser.c index d05d17c3..1309e8e1 100644 --- a/tests/test_parser.c +++ b/tests/test_parser.c @@ -502,4 +502,169 @@ get_first_two_of_three_first_and_second_quoted(void **state) char *result = get_start(inp, 3); assert_string_equal("\"one\" \"two\" ", result); +} + +void +parse_options_when_none_returns_empty_hasmap(void **state) +{ + gchar *args[] = { "cmd1", "cmd2", NULL }; + + GList *keys = NULL; + keys = g_list_append(keys, "opt1"); + + gboolean res = FALSE; + + GHashTable *options = parse_options(args, 2, keys, &res); + + assert_true(options != NULL); + assert_int_equal(0, g_hash_table_size(options)); + assert_true(res); + + options_destroy(options); +} + +void +parse_options_when_opt1_no_val_sets_error(void **state) +{ + gchar *args[] = { "cmd1", "cmd2", "opt1", NULL }; + + GList *keys = NULL; + keys = g_list_append(keys, "opt1"); + + gboolean res = TRUE; + + GHashTable *options = parse_options(args, 2, keys, &res); + + assert_null(options); + assert_false(res); + + options_destroy(options); +} + +void +parse_options_when_one_returns_map(void **state) +{ + gchar *args[] = { "cmd1", "cmd2", "opt1", "val1", NULL }; + + GList *keys = NULL; + keys = g_list_append(keys, "opt1"); + + gboolean res = FALSE; + + GHashTable *options = parse_options(args, 2, keys, &res); + + assert_int_equal(1, g_hash_table_size(options)); + assert_true(g_hash_table_contains(options, "opt1")); + assert_string_equal("val1", g_hash_table_lookup(options, "opt1")); + assert_true(res); + + options_destroy(options); +} + +void +parse_options_when_opt2_no_val_sets_error(void **state) +{ + gchar *args[] = { "cmd1", "cmd2", "opt1", "val1", "opt2", NULL }; + + GList *keys = NULL; + keys = g_list_append(keys, "opt1"); + keys = g_list_append(keys, "opt2"); + + gboolean res = TRUE; + + GHashTable *options = parse_options(args, 2, keys, &res); + + assert_null(options); + assert_false(res); + + options_destroy(options); +} + +void +parse_options_when_two_returns_map(void **state) +{ + gchar *args[] = { "cmd1", "cmd2", "opt1", "val1", "opt2", "val2", NULL }; + + GList *keys = NULL; + keys = g_list_append(keys, "opt1"); + keys = g_list_append(keys, "opt2"); + + gboolean res = FALSE; + + GHashTable *options = parse_options(args, 2, keys, &res); + + assert_int_equal(2, g_hash_table_size(options)); + assert_true(g_hash_table_contains(options, "opt1")); + assert_true(g_hash_table_contains(options, "opt2")); + assert_string_equal("val1", g_hash_table_lookup(options, "opt1")); + assert_string_equal("val2", g_hash_table_lookup(options, "opt2")); + assert_true(res); + + options_destroy(options); +} + +void +parse_options_when_opt3_no_val_sets_error(void **state) +{ + gchar *args[] = { "cmd1", "cmd2", "opt1", "val1", "opt2", "val2", "opt3", NULL }; + + GList *keys = NULL; + keys = g_list_append(keys, "opt1"); + keys = g_list_append(keys, "opt2"); + keys = g_list_append(keys, "opt3"); + + gboolean res = TRUE; + + GHashTable *options = parse_options(args, 2, keys, &res); + + assert_null(options); + assert_false(res); + + options_destroy(options); +} + +void +parse_options_when_three_returns_map(void **state) +{ + gchar *args[] = { "cmd1", "cmd2", "opt1", "val1", "opt2", "val2", "opt3", "val3", NULL }; + + GList *keys = NULL; + keys = g_list_append(keys, "opt1"); + keys = g_list_append(keys, "opt2"); + keys = g_list_append(keys, "opt3"); + + gboolean res = FALSE; + + GHashTable *options = parse_options(args, 2, keys, &res); + + assert_int_equal(3, g_hash_table_size(options)); + assert_true(g_hash_table_contains(options, "opt1")); + assert_true(g_hash_table_contains(options, "opt2")); + assert_true(g_hash_table_contains(options, "opt3")); + assert_string_equal("val1", g_hash_table_lookup(options, "opt1")); + assert_string_equal("val2", g_hash_table_lookup(options, "opt2")); + assert_string_equal("val3", g_hash_table_lookup(options, "opt3")); + assert_true(res); + + options_destroy(options); +} + +void +parse_options_when_unknown_opt_sets_error(void **state) +{ + gchar *args[] = { "cmd1", "cmd2", "opt1", "val1", "oops", "val2", "opt3", "val3", NULL }; + + GList *keys = NULL; + keys = g_list_append(keys, "opt1"); + keys = g_list_append(keys, "opt2"); + keys = g_list_append(keys, "opt3"); + + gboolean res = TRUE; + + GHashTable *options = parse_options(args, 2, keys, &res); + + assert_null(options); + assert_false(res); + + options_destroy(options); } \ No newline at end of file diff --git a/tests/test_parser.h b/tests/test_parser.h index 7b178c3d..70b94b50 100644 --- a/tests/test_parser.h +++ b/tests/test_parser.h @@ -39,3 +39,11 @@ void get_first_two_of_three(void **state); void get_first_two_of_three_first_quoted(void **state); void get_first_two_of_three_second_quoted(void **state); void get_first_two_of_three_first_and_second_quoted(void **state); +void parse_options_when_none_returns_empty_hasmap(void **state); +void parse_options_when_opt1_no_val_sets_error(void **state); +void parse_options_when_one_returns_map(void **state); +void parse_options_when_opt2_no_val_sets_error(void **state); +void parse_options_when_two_returns_map(void **state); +void parse_options_when_opt3_no_val_sets_error(void **state); +void parse_options_when_three_returns_map(void **state); +void parse_options_when_unknown_opt_sets_error(void **state); \ No newline at end of file diff --git a/tests/testsuite.c b/tests/testsuite.c index cdcd61ee..c6577410 100644 --- a/tests/testsuite.c +++ b/tests/testsuite.c @@ -165,6 +165,14 @@ int main(int argc, char* argv[]) { unit_test(get_first_two_of_three_first_quoted), unit_test(get_first_two_of_three_second_quoted), unit_test(get_first_two_of_three_first_and_second_quoted), + unit_test(parse_options_when_none_returns_empty_hasmap), + unit_test(parse_options_when_opt1_no_val_sets_error), + unit_test(parse_options_when_one_returns_map), + unit_test(parse_options_when_opt2_no_val_sets_error), + unit_test(parse_options_when_two_returns_map), + unit_test(parse_options_when_opt3_no_val_sets_error), + unit_test(parse_options_when_three_returns_map), + unit_test(parse_options_when_unknown_opt_sets_error), unit_test(empty_list_when_none_added), unit_test(contains_one_element), From 241973700697abb60104c656a0aacfd182eb2316 Mon Sep 17 00:00:00 2001 From: James Booth Date: Mon, 14 Apr 2014 23:01:57 +0100 Subject: [PATCH 3/6] Check for duplicate options in option parser --- src/tools/parser.c | 21 +++++++++++++++++++-- tests/test_parser.c | 20 ++++++++++++++++++++ tests/test_parser.h | 3 ++- tests/testsuite.c | 1 + 4 files changed, 42 insertions(+), 3 deletions(-) diff --git a/src/tools/parser.c b/src/tools/parser.c index 375572e0..b4403d06 100644 --- a/src/tools/parser.c +++ b/src/tools/parser.c @@ -385,14 +385,31 @@ parse_options(gchar **args, int start, GList *keys, gboolean *res) return options; } - // check each option is valid and has value, failure if not + // validate options int curr; + GList *found_keys = NULL; for (curr = start; curr < g_strv_length(args); curr+= 2) { - if ((g_list_find(keys, args[curr]) == NULL) || (args[curr+1] == NULL)) { + // check if option valid + if (g_list_find(keys, args[curr]) == NULL) { *res = FALSE; return options; } + + // check if duplicate + if (g_list_find(found_keys, args[curr]) != NULL) { + *res = FALSE; + return options; + } + + // check value given + if (args[curr+1] == NULL) { + *res = FALSE; + return options; + } + + found_keys = g_list_append(found_keys, args[curr]); } + g_list_free(found_keys); // create map options = g_hash_table_new(g_str_hash, g_str_equal); diff --git a/tests/test_parser.c b/tests/test_parser.c index 1309e8e1..6e1d4cde 100644 --- a/tests/test_parser.c +++ b/tests/test_parser.c @@ -666,5 +666,25 @@ parse_options_when_unknown_opt_sets_error(void **state) assert_null(options); assert_false(res); + options_destroy(options); +} + +void +parse_options_with_duplicated_option_sets_error(void **state) +{ + gchar *args[] = { "cmd1", "cmd2", "opt1", "val1", "opt2", "val2", "opt1", "val3", NULL }; + + GList *keys = NULL; + keys = g_list_append(keys, "opt1"); + keys = g_list_append(keys, "opt2"); + keys = g_list_append(keys, "opt3"); + + gboolean res = TRUE; + + GHashTable *options = parse_options(args, 2, keys, &res); + + assert_null(options); + assert_false(res); + options_destroy(options); } \ No newline at end of file diff --git a/tests/test_parser.h b/tests/test_parser.h index 70b94b50..51d768fe 100644 --- a/tests/test_parser.h +++ b/tests/test_parser.h @@ -46,4 +46,5 @@ void parse_options_when_opt2_no_val_sets_error(void **state); void parse_options_when_two_returns_map(void **state); void parse_options_when_opt3_no_val_sets_error(void **state); void parse_options_when_three_returns_map(void **state); -void parse_options_when_unknown_opt_sets_error(void **state); \ No newline at end of file +void parse_options_when_unknown_opt_sets_error(void **state); +void parse_options_with_duplicated_option_sets_error(void **state); diff --git a/tests/testsuite.c b/tests/testsuite.c index c6577410..05f03188 100644 --- a/tests/testsuite.c +++ b/tests/testsuite.c @@ -173,6 +173,7 @@ int main(int argc, char* argv[]) { unit_test(parse_options_when_opt3_no_val_sets_error), unit_test(parse_options_when_three_returns_map), unit_test(parse_options_when_unknown_opt_sets_error), + unit_test(parse_options_with_duplicated_option_sets_error), unit_test(empty_list_when_none_added), unit_test(contains_one_element), From 428d9eb936db2436ae72f2bf18afba635b15cbeb Mon Sep 17 00:00:00 2001 From: James Booth Date: Mon, 14 Apr 2014 23:15:39 +0100 Subject: [PATCH 4/6] Refactored cmd_connect to use parse_options --- src/command/commands.c | 95 ++++++++++-------------------------------- 1 file changed, 23 insertions(+), 72 deletions(-) diff --git a/src/command/commands.c b/src/command/commands.c index 0fc8574b..c03d294d 100644 --- a/src/command/commands.c +++ b/src/command/commands.c @@ -68,85 +68,36 @@ cmd_connect(gchar **args, struct cmd_help_t help) cons_show("You are either connected already, or a login is in process."); result = TRUE; } else { - char *user = args[0]; - char *opt1 = args[1]; - char *opt1val = args[2]; - char *opt2 = args[3]; - char *opt2val = args[4]; - char *lower = g_utf8_strdown(user, -1); - char *jid; + GList *opt_keys = NULL; + opt_keys = g_list_append(opt_keys, "server"); + opt_keys = g_list_append(opt_keys, "port"); + gboolean parsed; + + GHashTable *options = parse_options(args, 1, opt_keys, &parsed); + if (!parsed) { + cons_show("Usage: %s", help.usage); + cons_show(""); + return TRUE; + } + + char *altdomain = g_hash_table_lookup(options, "server"); - // parse options - char *altdomain = NULL; int port = 0; - gboolean server_set = FALSE; - gboolean port_set = FALSE; - if (opt1 != NULL) { - if (opt1val == NULL) { - cons_show("Usage: %s", help.usage); + if (g_hash_table_contains(options, "port")) { + char *port_str = g_hash_table_lookup(options, "port"); + if (_strtoi(port_str, &port, 1, 65535) != 0) { + port = 0; cons_show(""); return TRUE; } - if (strcmp(opt1, "server") == 0) { - altdomain = opt1val; - server_set = TRUE; - } else if (strcmp(opt1, "port") == 0) { - if (_strtoi(opt1val, &port, 1, 65535) != 0) { - port = 0; - cons_show(""); - return TRUE; - } else { - port_set = TRUE; - } - } else { - cons_show("Usage: %s", help.usage); - cons_show(""); - return TRUE; - } - - if (opt2 != NULL) { - if (server_set && strcmp("server", opt2) == 0) { - cons_show("Usage: %s", help.usage); - cons_show(""); - return TRUE; - } - if (port_set && strcmp("port", opt2) == 0) { - cons_show("Usage: %s", help.usage); - cons_show(""); - return TRUE; - } - if (opt2val == NULL) { - cons_show("Usage: %s", help.usage); - cons_show(""); - return TRUE; - } - if (strcmp(opt2, "server") == 0) { - if (server_set) { - cons_show("Usage: %s", help.usage); - return TRUE; - } - altdomain = opt2val; - server_set = TRUE; - } else if (strcmp(opt2, "port") == 0) { - if (port_set) { - cons_show("Usage: %s", help.usage); - return TRUE; - } - if (_strtoi(opt2val, &port, 1, 65535) != 0) { - port = 0; - cons_show(""); - return TRUE; - } else { - port_set = TRUE; - } - } else { - cons_show("Usage: %s", help.usage); - cons_show(""); - return TRUE; - } - } } + options_destroy(options); + + char *user = args[0]; + char *lower = g_utf8_strdown(user, -1); + char *jid; + ProfAccount *account = accounts_get_account(lower); if (account != NULL) { jid = account_create_full_jid(account); From 3e69d6b71ed57964df92f172d8f7a041e798a2e3 Mon Sep 17 00:00:00 2001 From: James Booth Date: Mon, 14 Apr 2014 23:36:00 +0100 Subject: [PATCH 5/6] Refactored cmd_join to use parse_options --- src/command/commands.c | 48 +++++++++++++----------------------------- src/tools/parser.c | 4 ++-- 2 files changed, 17 insertions(+), 35 deletions(-) diff --git a/src/command/commands.c b/src/command/commands.c index c03d294d..51334934 100644 --- a/src/command/commands.c +++ b/src/command/commands.c @@ -1551,7 +1551,6 @@ cmd_join(gchar **args, struct cmd_help_t help) return TRUE; } - int num_args = g_strv_length(args); char *room = NULL; char *nick = NULL; char *passwd = NULL; @@ -1572,40 +1571,23 @@ cmd_join(gchar **args, struct cmd_help_t help) } // Additional args supplied - if (num_args > 1) { - char *opt1 = args[1]; - char *opt1val = args[2]; - char *opt2 = args[3]; - char *opt2val = args[4]; - if (opt1 != NULL) { - if (opt1val == NULL) { - cons_show("Usage: %s", help.usage); - cons_show(""); - return TRUE; - } - if (strcmp(opt1, "nick") == 0) { - nick = opt1val; - } else if (strcmp(opt1, "password") == 0) { - passwd = opt1val; - } else { - cons_show("Usage: %s", help.usage); - cons_show(""); - return TRUE; - } - if (opt2 != NULL) { - if (strcmp(opt2, "nick") == 0) { - nick = opt2val; - } else if (strcmp(opt2, "password") == 0) { - passwd = opt2val; - } else { - cons_show("Usage: %s", help.usage); - cons_show(""); - return TRUE; - } - } - } + GList *opt_keys = NULL; + opt_keys = g_list_append(opt_keys, "nick"); + opt_keys = g_list_append(opt_keys, "password"); + gboolean parsed; + + GHashTable *options = parse_options(args, 1, opt_keys, &parsed); + if (!parsed) { + cons_show("Usage: %s", help.usage); + cons_show(""); + return TRUE; } + nick = g_hash_table_lookup(options, "nick"); + passwd = g_hash_table_lookup(options, "password"); + + options_destroy(options); + // In the case that a nick wasn't provided by the optional args... if (nick == NULL) { nick = account->muc_nick; diff --git a/src/tools/parser.c b/src/tools/parser.c index b4403d06..6db522aa 100644 --- a/src/tools/parser.c +++ b/src/tools/parser.c @@ -390,13 +390,13 @@ parse_options(gchar **args, int start, GList *keys, gboolean *res) GList *found_keys = NULL; for (curr = start; curr < g_strv_length(args); curr+= 2) { // check if option valid - if (g_list_find(keys, args[curr]) == NULL) { + if (g_list_find_custom(keys, args[curr], (GCompareFunc)g_strcmp0) == NULL) { *res = FALSE; return options; } // check if duplicate - if (g_list_find(found_keys, args[curr]) != NULL) { + if (g_list_find_custom(found_keys, args[curr], (GCompareFunc)g_strcmp0) != NULL) { *res = FALSE; return options; } From 79088d01500ba5f6224a8d49a45ed1e1bc96ef07 Mon Sep 17 00:00:00 2001 From: James Booth Date: Mon, 14 Apr 2014 23:41:45 +0100 Subject: [PATCH 6/6] Clean up keys after using parse_options --- src/command/commands.c | 2 ++ tests/test_parser.c | 9 +++++++++ 2 files changed, 11 insertions(+) diff --git a/src/command/commands.c b/src/command/commands.c index 51334934..63ecb48e 100644 --- a/src/command/commands.c +++ b/src/command/commands.c @@ -93,6 +93,7 @@ cmd_connect(gchar **args, struct cmd_help_t help) } options_destroy(options); + g_list_free(opt_keys); char *user = args[0]; char *lower = g_utf8_strdown(user, -1); @@ -1587,6 +1588,7 @@ cmd_join(gchar **args, struct cmd_help_t help) passwd = g_hash_table_lookup(options, "password"); options_destroy(options); + g_list_free(opt_keys); // In the case that a nick wasn't provided by the optional args... if (nick == NULL) { diff --git a/tests/test_parser.c b/tests/test_parser.c index 6e1d4cde..df7836a0 100644 --- a/tests/test_parser.c +++ b/tests/test_parser.c @@ -521,6 +521,7 @@ parse_options_when_none_returns_empty_hasmap(void **state) assert_true(res); options_destroy(options); + g_list_free(keys); } void @@ -539,6 +540,7 @@ parse_options_when_opt1_no_val_sets_error(void **state) assert_false(res); options_destroy(options); + g_list_free(keys); } void @@ -559,6 +561,7 @@ parse_options_when_one_returns_map(void **state) assert_true(res); options_destroy(options); + g_list_free(keys); } void @@ -578,6 +581,7 @@ parse_options_when_opt2_no_val_sets_error(void **state) assert_false(res); options_destroy(options); + g_list_free(keys); } void @@ -601,6 +605,7 @@ parse_options_when_two_returns_map(void **state) assert_true(res); options_destroy(options); + g_list_free(keys); } void @@ -621,6 +626,7 @@ parse_options_when_opt3_no_val_sets_error(void **state) assert_false(res); options_destroy(options); + g_list_free(keys); } void @@ -647,6 +653,7 @@ parse_options_when_three_returns_map(void **state) assert_true(res); options_destroy(options); + g_list_free(keys); } void @@ -667,6 +674,7 @@ parse_options_when_unknown_opt_sets_error(void **state) assert_false(res); options_destroy(options); + g_list_free(keys); } void @@ -687,4 +695,5 @@ parse_options_with_duplicated_option_sets_error(void **state) assert_false(res); options_destroy(options); + g_list_free(keys); } \ No newline at end of file