From c53b78ea1c115ff3e541debb772221b40aab422e Mon Sep 17 00:00:00 2001 From: Steffen Jaeckel Date: Thu, 31 Mar 2022 10:33:14 +0200 Subject: [PATCH 1/4] use `_string_matches_one_of()` at more places I missed them the last time ... Signed-off-by: Steffen Jaeckel --- src/command/cmd_funcs.c | 23 +++++++---------------- 1 file changed, 7 insertions(+), 16 deletions(-) diff --git a/src/command/cmd_funcs.c b/src/command/cmd_funcs.c index 43ec3a0c..8c454099 100644 --- a/src/command/cmd_funcs.c +++ b/src/command/cmd_funcs.c @@ -1781,7 +1781,7 @@ _who_room(ProfWin* window, const char* const command, gchar** args) } // bad arg - if (args[0] && (g_strcmp0(args[0], "online") != 0) && (g_strcmp0(args[0], "available") != 0) && (g_strcmp0(args[0], "unavailable") != 0) && (g_strcmp0(args[0], "away") != 0) && (g_strcmp0(args[0], "chat") != 0) && (g_strcmp0(args[0], "xa") != 0) && (g_strcmp0(args[0], "dnd") != 0) && (g_strcmp0(args[0], "any") != 0) && (g_strcmp0(args[0], "moderator") != 0) && (g_strcmp0(args[0], "participant") != 0) && (g_strcmp0(args[0], "visitor") != 0) && (g_strcmp0(args[0], "owner") != 0) && (g_strcmp0(args[0], "admin") != 0) && (g_strcmp0(args[0], "member") != 0) && (g_strcmp0(args[0], "outcast") != 0) && (g_strcmp0(args[0], "none") != 0)) { + if (!_string_matches_one_of(args[0], FALSE, "online", "available", "unavailable", "away", "chat", "xa", "dnd", "any", "moderator", "participant", "visitor", "owner", "admin", "member", "outcast", "none", NULL)) { cons_bad_cmd_usage(command); return; } @@ -1790,7 +1790,7 @@ _who_room(ProfWin* window, const char* const command, gchar** args) assert(mucwin->memcheck == PROFMUCWIN_MEMCHECK); // presence filter - if (args[0] == NULL || (g_strcmp0(args[0], "online") == 0) || (g_strcmp0(args[0], "available") == 0) || (g_strcmp0(args[0], "unavailable") == 0) || (g_strcmp0(args[0], "away") == 0) || (g_strcmp0(args[0], "chat") == 0) || (g_strcmp0(args[0], "xa") == 0) || (g_strcmp0(args[0], "dnd") == 0) || (g_strcmp0(args[0], "any") == 0)) { + if (_string_matches_one_of(args[0], TRUE, "online", "available", "unavailable", "away", "chat", "xa", "dnd", "any", NULL)) { char* presence = args[0]; GList* occupants = muc_roster(mucwin->roomjid); @@ -1889,16 +1889,7 @@ _who_roster(ProfWin* window, const char* const command, gchar** args) char* presence = args[0]; // bad arg - if (presence - && (strcmp(presence, "online") != 0) - && (strcmp(presence, "available") != 0) - && (strcmp(presence, "unavailable") != 0) - && (strcmp(presence, "offline") != 0) - && (strcmp(presence, "away") != 0) - && (strcmp(presence, "chat") != 0) - && (strcmp(presence, "xa") != 0) - && (strcmp(presence, "dnd") != 0) - && (strcmp(presence, "any") != 0)) { + if (!_string_matches_one_of(presence, TRUE, "online", "available", "unavailable", "offline", "away", "chat", "xa", "dnd", "any", NULL)) { cons_bad_cmd_usage(command); return; } @@ -3952,7 +3943,7 @@ cmd_form(ProfWin* window, const char* const command, gchar** args) return TRUE; } - if ((g_strcmp0(args[0], "submit") != 0) && (g_strcmp0(args[0], "cancel") != 0) && (g_strcmp0(args[0], "show") != 0) && (g_strcmp0(args[0], "help") != 0)) { + if (!_string_matches_one_of(args[0], FALSE, "submit", "cancel", "show", "help", NULL)) { cons_bad_cmd_usage(command); return TRUE; } @@ -5260,7 +5251,7 @@ cmd_console(ProfWin* window, const char* const command, gchar** args) { gboolean isMuc = (g_strcmp0(args[0], "muc") == 0); - if ((g_strcmp0(args[0], "chat") != 0) && !isMuc && (g_strcmp0(args[0], "private") != 0)) { + if (!_string_matches_one_of(args[0], "chat", "private", NULL) && !isMuc) { cons_bad_cmd_usage(command); return TRUE; } @@ -6576,13 +6567,13 @@ cmd_ping(ProfWin* window, const char* const command, gchar** args) gboolean cmd_autoaway(ProfWin* window, const char* const command, gchar** args) { - if ((g_strcmp0(args[0], "mode") != 0) && (g_strcmp0(args[0], "time") != 0) && (g_strcmp0(args[0], "message") != 0) && (g_strcmp0(args[0], "check") != 0)) { + if (!_string_matches_one_of(args[0], FALSE, "mode", "time", "message", "check", NULL)) { cons_show("Setting must be one of 'mode', 'time', 'message' or 'check'"); return TRUE; } if (g_strcmp0(args[0], "mode") == 0) { - if ((g_strcmp0(args[1], "idle") != 0) && (g_strcmp0(args[1], "away") != 0) && (g_strcmp0(args[1], "off") != 0)) { + if (!_string_matches_one_of(args[1], FALSE, "idle", "away", "off", NULL)) { cons_show("Mode must be one of 'idle', 'away' or 'off'"); } else { prefs_set_string(PREF_AUTOAWAY_MODE, args[1]); From f284641710bd25232676893327b5d46968ca481b Mon Sep 17 00:00:00 2001 From: Steffen Jaeckel Date: Thu, 31 Mar 2022 11:22:14 +0200 Subject: [PATCH 2/4] less duplication Print error message from `_string_matches_one_of()` instead of forming an error message manually that contains the same entries that were checked in `_string_matches_one_of()`. Signed-off-by: Steffen Jaeckel --- src/command/cmd_funcs.c | 92 ++++++++++++++++++------------ tests/unittests/test_cmd_account.c | 3 +- 2 files changed, 58 insertions(+), 37 deletions(-) diff --git a/src/command/cmd_funcs.c b/src/command/cmd_funcs.c index 8c454099..4fd9d20b 100644 --- a/src/command/cmd_funcs.c +++ b/src/command/cmd_funcs.c @@ -125,10 +125,10 @@ static gboolean _cmd_execute(ProfWin* window, const char* const command, const c static gboolean _cmd_execute_default(ProfWin* window, const char* inp); static gboolean _cmd_execute_alias(ProfWin* window, const char* const inp, gboolean* ran); static gboolean -_string_matches_one_of(const char* is, bool is_can_be_null, const char* first, ...) __attribute__((sentinel)); +_string_matches_one_of(const char* what, const char* is, bool is_can_be_null, const char* first, ...) __attribute__((sentinel)); static gboolean -_string_matches_one_of(const char* is, bool is_can_be_null, const char* first, ...) +_string_matches_one_of(const char* what, const char* is, bool is_can_be_null, const char* first, ...) { gboolean ret = FALSE; va_list ap; @@ -136,10 +136,8 @@ _string_matches_one_of(const char* is, bool is_can_be_null, const char* first, . if (!is) return is_can_be_null; - log_debug("%s vs.", is); va_start(ap, first); while (cur != NULL) { - log_debug("%s", cur); if (g_strcmp0(is, cur) == 0) { ret = TRUE; break; @@ -147,6 +145,39 @@ _string_matches_one_of(const char* is, bool is_can_be_null, const char* first, . cur = va_arg(ap, const char*); } va_end(ap); + if (!ret && what) { + cons_show("Invalid %s: '%s'", what, is); + char errmsg[256] = { 0 }; + size_t sz = 0; + int s = snprintf(errmsg, sizeof(errmsg) - sz, "%s must be one of:", what); + if (s < 0 || s + sz >= sizeof(errmsg)) + return ret; + sz += s; + + cur = first; + va_start(ap, first); + while (cur != NULL) { + const char* next = va_arg(ap, const char*); + if (next) { + s = snprintf(errmsg + sz, sizeof(errmsg) - sz, " '%s',", cur); + } else { + /* remove last ',' */ + sz--; + errmsg[sz] = '\0'; + s = snprintf(errmsg + sz, sizeof(errmsg) - sz, " or '%s'.", cur); + } + if (s < 0 || s + sz >= sizeof(errmsg)) { + log_debug("Error message too long or some other error occurred (%d).", s); + s = -1; + break; + } + sz += s; + cur = next; + } + va_end(ap); + if (s > 0) + cons_show(errmsg); + } return ret; } @@ -364,7 +395,7 @@ cmd_connect(ProfWin* window, const char* const command, gchar** args) char* altdomain = g_hash_table_lookup(options, "server"); char* tls_policy = g_hash_table_lookup(options, "tls"); - if (!_string_matches_one_of(tls_policy, TRUE, "force", "allow", "trust", "disable", "legacy", NULL)) { + if (!_string_matches_one_of("TLS policy", tls_policy, TRUE, "force", "allow", "trust", "disable", "legacy", NULL)) { cons_bad_cmd_usage(command); cons_show(""); options_destroy(options); @@ -372,7 +403,7 @@ cmd_connect(ProfWin* window, const char* const command, gchar** args) } char* auth_policy = g_hash_table_lookup(options, "auth"); - if (!_string_matches_one_of(auth_policy, TRUE, "default", "legacy", NULL)) { + if (!_string_matches_one_of("Auth policy", auth_policy, TRUE, "default", "legacy", NULL)) { cons_bad_cmd_usage(command); cons_show(""); options_destroy(options); @@ -757,9 +788,7 @@ _account_set_nick(char* account_name, char* nick) gboolean _account_set_otr(char* account_name, char* policy) { - if (!_string_matches_one_of(policy, FALSE, "manual", "opportunistic", "always", NULL)) { - cons_show("OTR policy must be one of: manual, opportunistic or always."); - } else { + if (_string_matches_one_of("OTR policy", policy, FALSE, "manual", "opportunistic", "always", NULL)) { accounts_set_otr_policy(account_name, policy); cons_show("Updated OTR policy for account %s: %s", account_name, policy); cons_show(""); @@ -844,9 +873,7 @@ _account_set_theme(char* account_name, char* theme) gboolean _account_set_tls(char* account_name, char* policy) { - if (!_string_matches_one_of(policy, FALSE, "force", "allow", "trust", "disable", "legacy", NULL)) { - cons_show("TLS policy must be one of: force, allow, legacy or disable."); - } else { + if (_string_matches_one_of("TLS policy", policy, FALSE, "force", "allow", "trust", "disable", "legacy", NULL)) { accounts_set_tls_policy(account_name, policy); cons_show("Updated TLS policy for account %s: %s", account_name, policy); cons_show(""); @@ -857,9 +884,7 @@ _account_set_tls(char* account_name, char* policy) gboolean _account_set_auth(char* account_name, char* policy) { - if (!_string_matches_one_of(policy, FALSE, "default", "legacy", NULL)) { - cons_show("Auth policy must be either default or legacy."); - } else { + if (_string_matches_one_of("Auth policy", policy, FALSE, "default", "legacy", NULL)) { accounts_set_auth_policy(account_name, policy); cons_show("Updated auth policy for account %s: %s", account_name, policy); cons_show(""); @@ -1781,7 +1806,7 @@ _who_room(ProfWin* window, const char* const command, gchar** args) } // bad arg - if (!_string_matches_one_of(args[0], FALSE, "online", "available", "unavailable", "away", "chat", "xa", "dnd", "any", "moderator", "participant", "visitor", "owner", "admin", "member", "outcast", "none", NULL)) { + if (!_string_matches_one_of(NULL, args[0], TRUE, "online", "available", "unavailable", "away", "chat", "xa", "dnd", "any", "moderator", "participant", "visitor", "owner", "admin", "member", "outcast", "none", NULL)) { cons_bad_cmd_usage(command); return; } @@ -1790,7 +1815,7 @@ _who_room(ProfWin* window, const char* const command, gchar** args) assert(mucwin->memcheck == PROFMUCWIN_MEMCHECK); // presence filter - if (_string_matches_one_of(args[0], TRUE, "online", "available", "unavailable", "away", "chat", "xa", "dnd", "any", NULL)) { + if (_string_matches_one_of(NULL, args[0], TRUE, "online", "available", "unavailable", "away", "chat", "xa", "dnd", "any", NULL)) { char* presence = args[0]; GList* occupants = muc_roster(mucwin->roomjid); @@ -1889,7 +1914,7 @@ _who_roster(ProfWin* window, const char* const command, gchar** args) char* presence = args[0]; // bad arg - if (!_string_matches_one_of(presence, TRUE, "online", "available", "unavailable", "offline", "away", "chat", "xa", "dnd", "any", NULL)) { + if (!_string_matches_one_of(NULL, presence, TRUE, "online", "available", "unavailable", "offline", "away", "chat", "xa", "dnd", "any", NULL)) { cons_bad_cmd_usage(command); return; } @@ -3782,7 +3807,7 @@ cmd_form_field(ProfWin* window, char* tag, gchar** args) if (cmd) { value = args[1]; } - if (!_string_matches_one_of(cmd, FALSE, "add", "remove", NULL)) { + if (!_string_matches_one_of(NULL, cmd, FALSE, "add", "remove", NULL)) { win_println(window, THEME_DEFAULT, "-", "Invalid command, usage:"); confwin_field_help(confwin, tag); win_println(window, THEME_DEFAULT, "-", ""); @@ -3836,7 +3861,7 @@ cmd_form_field(ProfWin* window, char* tag, gchar** args) if (cmd) { value = args[1]; } - if (!_string_matches_one_of(cmd, FALSE, "add", "remove", NULL)) { + if (!_string_matches_one_of(NULL, cmd, FALSE, "add", "remove", NULL)) { win_println(window, THEME_DEFAULT, "-", "Invalid command, usage:"); confwin_field_help(confwin, tag); win_println(window, THEME_DEFAULT, "-", ""); @@ -3887,7 +3912,7 @@ cmd_form_field(ProfWin* window, char* tag, gchar** args) if (cmd) { value = args[1]; } - if (!_string_matches_one_of(cmd, FALSE, "add", "remove", NULL)) { + if (!_string_matches_one_of(NULL, cmd, FALSE, "add", "remove", NULL)) { win_println(window, THEME_DEFAULT, "-", "Invalid command, usage:"); confwin_field_help(confwin, tag); win_println(window, THEME_DEFAULT, "-", ""); @@ -3943,7 +3968,7 @@ cmd_form(ProfWin* window, const char* const command, gchar** args) return TRUE; } - if (!_string_matches_one_of(args[0], FALSE, "submit", "cancel", "show", "help", NULL)) { + if (!_string_matches_one_of(NULL, args[0], FALSE, "submit", "cancel", "show", "help", NULL)) { cons_bad_cmd_usage(command); return TRUE; } @@ -4192,7 +4217,7 @@ cmd_affiliation(ProfWin* window, const char* const command, gchar** args) } char* affiliation = args[1]; - if (!_string_matches_one_of(affiliation, TRUE, "owner", "admin", "member", "none", "outcast", NULL)) { + if (!_string_matches_one_of(NULL, affiliation, TRUE, "owner", "admin", "member", "none", "outcast", NULL)) { cons_bad_cmd_usage(command); return TRUE; } @@ -4267,7 +4292,7 @@ cmd_role(ProfWin* window, const char* const command, gchar** args) } char* role = args[1]; - if (!_string_matches_one_of(role, TRUE, "visitor", "participant", "moderator", "none", NULL)) { + if (!_string_matches_one_of(NULL, role, TRUE, "visitor", "participant", "moderator", "none", NULL)) { cons_bad_cmd_usage(command); return TRUE; } @@ -5251,13 +5276,13 @@ cmd_console(ProfWin* window, const char* const command, gchar** args) { gboolean isMuc = (g_strcmp0(args[0], "muc") == 0); - if (!_string_matches_one_of(args[0], "chat", "private", NULL) && !isMuc) { + if (!_string_matches_one_of(NULL, args[0], FALSE, "chat", "private", NULL) && !isMuc) { cons_bad_cmd_usage(command); return TRUE; } gchar* setting = args[1]; - if (!_string_matches_one_of(setting, FALSE, "all", "first", "none", NULL)) { + if (!_string_matches_one_of(NULL, setting, FALSE, "all", "first", "none", NULL)) { if (!(isMuc && (g_strcmp0(setting, "mention") == 0))) { cons_bad_cmd_usage(command); return TRUE; @@ -6567,15 +6592,12 @@ cmd_ping(ProfWin* window, const char* const command, gchar** args) gboolean cmd_autoaway(ProfWin* window, const char* const command, gchar** args) { - if (!_string_matches_one_of(args[0], FALSE, "mode", "time", "message", "check", NULL)) { - cons_show("Setting must be one of 'mode', 'time', 'message' or 'check'"); + if (!_string_matches_one_of("Setting", args[0], FALSE, "mode", "time", "message", "check", NULL)) { return TRUE; } if (g_strcmp0(args[0], "mode") == 0) { - if (!_string_matches_one_of(args[1], FALSE, "idle", "away", "off", NULL)) { - cons_show("Mode must be one of 'idle', 'away' or 'off'"); - } else { + if (_string_matches_one_of("Mode", args[1], FALSE, "idle", "away", "off", NULL)) { prefs_set_string(PREF_AUTOAWAY_MODE, args[1]); cons_show("Auto away mode set to: %s.", args[1]); } @@ -7761,8 +7783,7 @@ cmd_otr_policy(ProfWin* window, const char* const command, gchar** args) } char* choice = args[1]; - if (!_string_matches_one_of(choice, FALSE, "manual", "opportunistic", "always", NULL)) { - cons_show("OTR policy can be set to: manual, opportunistic or always."); + if (!_string_matches_one_of("OTR policy", choice, FALSE, "manual", "opportunistic", "always", NULL)) { return TRUE; } @@ -8951,8 +8972,7 @@ cmd_omemo_policy(ProfWin* window, const char* const command, gchar** args) } char* choice = args[1]; - if (!_string_matches_one_of(choice, FALSE, "manual", "automatic", "always", NULL)) { - cons_show("OMEMO policy can be set to: manual, automatic or always."); + if (!_string_matches_one_of("OMEMO policy", choice, FALSE, "manual", "automatic", "always", NULL)) { return TRUE; } @@ -9565,7 +9585,7 @@ cmd_register(ProfWin* window, const char* const command, gchar** args) } char* tls_policy = g_hash_table_lookup(options, "tls"); - if (!_string_matches_one_of(tls_policy, TRUE, "force", "allow", "trust", "disable", "legacy", NULL)) { + if (!_string_matches_one_of("TLS policy", tls_policy, TRUE, "force", "allow", "trust", "disable", "legacy", NULL)) { cons_bad_cmd_usage(command); cons_show(""); options_destroy(options); diff --git a/tests/unittests/test_cmd_account.c b/tests/unittests/test_cmd_account.c index 69721311..c0eac0e3 100644 --- a/tests/unittests/test_cmd_account.c +++ b/tests/unittests/test_cmd_account.c @@ -571,7 +571,8 @@ cmd_account_show_message_for_invalid_otr_policy(void** state) expect_any(accounts_account_exists, account_name); will_return(accounts_account_exists, TRUE); - expect_cons_show("OTR policy must be one of: manual, opportunistic or always."); + expect_cons_show("Invalid OTR policy: 'bad_otr_policy'"); + expect_cons_show("OTR policy must be one of: 'manual', 'opportunistic' or 'always'."); gboolean result = cmd_account_set(NULL, CMD_ACCOUNT, args); assert_true(result); From 1c7bae4ae99e6eec39df603fbf11c05e014ac21f Mon Sep 17 00:00:00 2001 From: Steffen Jaeckel Date: Thu, 31 Mar 2022 12:40:55 +0200 Subject: [PATCH 3/4] fix linter warnings Signed-off-by: Steffen Jaeckel --- src/ui/window.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/ui/window.c b/src/ui/window.c index 353b72ff..5ba2281d 100644 --- a/src/ui/window.c +++ b/src/ui/window.c @@ -415,6 +415,7 @@ win_get_last_sent_message(ProfWin* window) ProfMucWin* mucwin = (ProfMucWin*)window; assert(mucwin->memcheck == PROFMUCWIN_MEMCHECK); last_message = mucwin->last_message; + break; } default: break; @@ -1578,7 +1579,7 @@ _win_print_internal(ProfWin* window, const char* show_char, int pad_indent, GDat char* color_pref = prefs_get_string(PREF_COLOR_NICK); if (color_pref != NULL && (strcmp(color_pref, "false") != 0)) { - if (flags & NO_ME || (!(flags & NO_ME) && prefs_get_boolean(PREF_COLOR_NICK_OWN))) { + if ((flags & NO_ME) || (!(flags & NO_ME) && prefs_get_boolean(PREF_COLOR_NICK_OWN))) { colour = theme_hash_attrs(from); } } From b914929320f9f13981218f4bb2b685059e2b854b Mon Sep 17 00:00:00 2001 From: Steffen Jaeckel Date: Fri, 1 Apr 2022 11:03:28 +0200 Subject: [PATCH 4/4] fix `account set theme` help Signed-off-by: Steffen Jaeckel --- src/command/cmd_defs.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/command/cmd_defs.c b/src/command/cmd_defs.c index 24d4a737..281d21c6 100644 --- a/src/command/cmd_defs.c +++ b/src/command/cmd_defs.c @@ -2111,7 +2111,7 @@ static struct cmd_t command_defs[] = { { "set tls disable", "Disable TLS for the connection." }, { "set auth default", "Use default authentication process." }, { "set auth legacy", "Allow legacy authentication." }, - { "set ", "Set the UI theme for the account." }, + { "set theme ", "Set the UI theme for the account." }, { "clear server", "Remove the server setting for this account." }, { "clear port", "Remove the port setting for this account." }, { "clear password", "Remove the password setting for this account." },