diff --git a/include/maxscale/modulecmd.h b/include/maxscale/modulecmd.h index 7c1b0088c..bfa8d6d5e 100644 --- a/include/maxscale/modulecmd.h +++ b/include/maxscale/modulecmd.h @@ -110,7 +110,8 @@ typedef struct modulecmd char *identifier; /**< Unique identifier */ char *domain; /**< Command domain */ MODULECMDFN func; /**< The registered function */ - int arg_count; /**< Number of arguments */ + int arg_count_min; /**< Minimum number of arguments */ + int arg_count_max; /**< Maximum number of arguments */ modulecmd_arg_type_t *arg_types; /**< Argument types */ struct modulecmd *next; /**< Next command */ } MODULECMD; @@ -168,4 +169,22 @@ void modulecmd_arg_free(MODULECMD_ARG *arg); */ bool modulecmd_call_command(const MODULECMD *cmd, const MODULECMD_ARG *args); +/** + * @brief Set the current error message + * + * Modules that register commands should use this function to report errors. + * This will overwrite the existing error message. + * + * @param format Format string + * @param ... Format string arguments + */ +void modulecmd_set_error(const char *format, ...); + +/** + * @brief Get the latest error generated by the modulecmd system + * + * @return Human-readable error message + */ +const char* modulecmd_get_error(); + MXS_END_DECLS diff --git a/server/core/config.c b/server/core/config.c index f2aec6ab3..3e62de294 100644 --- a/server/core/config.c +++ b/server/core/config.c @@ -2010,7 +2010,7 @@ config_truth_value(char *str) { return 0; } - MXS_ERROR("Not a boolean value: %s", str); + return -1; } diff --git a/server/core/modulecmd.c b/server/core/modulecmd.c index cf5d4515e..045f0b2e7 100644 --- a/server/core/modulecmd.c +++ b/server/core/modulecmd.c @@ -14,8 +14,18 @@ #include #include #include +#include #include +/** Size of the error buffer */ +#define MODULECMD_ERRBUF_SIZE 512 + +/** Thread local error buffer */ +thread_local char *errbuf = NULL; + +/** Parameter passed to functions that do not always expect arguments */ +static const MODULECMD_ARG MODULECMD_NO_ARGUMENTS = {0, NULL}; + /** * A registered domain */ @@ -34,6 +44,41 @@ typedef struct modulecmd_domain static MODULECMD_DOMAIN *modulecmd_domains = NULL; static SPINLOCK modulecmd_lock = SPINLOCK_INIT; +static inline void prepare_error() +{ + if (errbuf == NULL) + { + errbuf = MXS_MALLOC(MODULECMD_ERRBUF_SIZE); + MXS_ABORT_IF_NULL(errbuf); + errbuf[0] = '\0'; + } +} + +/** + * @brief Reset error message + * + * This should be the first function called in every API function that can + * generate errors. + */ +static void reset_error() +{ + prepare_error(); + errbuf[0] = '\0'; +} + +static void report_argc_mismatch(const MODULECMD *cmd, int argc) +{ + if (cmd->arg_count_min == cmd->arg_count_max) + { + modulecmd_set_error("Expected %d arguments, got %d.", cmd->arg_count_min, argc); + } + else + { + modulecmd_set_error("Expected between %d and %d arguments, got %d.", cmd->arg_count_min, cmd->arg_count_max, + argc); + } +} + static MODULECMD_DOMAIN* domain_create(const char *domain) { MODULECMD_DOMAIN *rval = MXS_MALLOC(sizeof(*rval)); @@ -97,8 +142,14 @@ static MODULECMD* command_create(const char *identifier, const char *domain, if (rval && id && dm && types) { + int argc_min = 0; + for (int i = 0; i < argc; i++) { + if (MODULECMD_ARG_IS_REQUIRED(argv[i])) + { + argc_min++; + } types[i] = argv[i]; } @@ -106,7 +157,8 @@ static MODULECMD* command_create(const char *identifier, const char *domain, rval->identifier = id; rval->domain = dm; rval->arg_types = types; - rval->arg_count = argc; + rval->arg_count_min = argc_min; + rval->arg_count_max = argc; rval->next = NULL; } else @@ -145,7 +197,7 @@ static bool domain_has_command(MODULECMD_DOMAIN *dm, const char *id) } static bool process_argument(modulecmd_arg_type_t type, const char* value, - struct arg_node *arg) + struct arg_node *arg, const char **err) { bool rval = false; @@ -169,19 +221,27 @@ static bool process_argument(modulecmd_arg_type_t type, const char* value, arg->type = MODULECMD_ARG_STRING; rval = true; } + else + { + *err = "memory allocation failed"; + } break; case MODULECMD_ARG_BOOLEAN: - { - int truthval = config_truth_value((char*)value); - if (truthval != -1) { - arg->value.boolean = truthval; - arg->type = MODULECMD_ARG_BOOLEAN; - rval = true; + int truthval = config_truth_value((char*)value); + if (truthval != -1) + { + arg->value.boolean = truthval; + arg->type = MODULECMD_ARG_BOOLEAN; + rval = true; + } + else + { + *err = "not a boolean value"; + } } - } - break; + break; case MODULECMD_ARG_SERVICE: if ((arg->value.service = service_find((char*)value))) @@ -189,6 +249,10 @@ static bool process_argument(modulecmd_arg_type_t type, const char* value, arg->type = MODULECMD_ARG_SERVICE; rval = true; } + else + { + *err = "service not found"; + } break; case MODULECMD_ARG_SERVER: @@ -197,6 +261,10 @@ static bool process_argument(modulecmd_arg_type_t type, const char* value, arg->type = MODULECMD_ARG_SERVER; rval = true; } + else + { + *err = "server not found"; + } break; case MODULECMD_ARG_SESSION: @@ -213,6 +281,10 @@ static bool process_argument(modulecmd_arg_type_t type, const char* value, arg->type = MODULECMD_ARG_MONITOR; rval = true; } + else + { + *err = "monitor not found"; + } break; case MODULECMD_ARG_FILTER: @@ -221,14 +293,23 @@ static bool process_argument(modulecmd_arg_type_t type, const char* value, arg->type = MODULECMD_ARG_FILTER; rval = true; } + else + { + *err = "filter not found"; + } break; default: ss_dassert(false); MXS_ERROR("Undefined argument type: %0lx", type); + *err = "internal error"; break; } } + else + { + *err = "required argument"; + } return rval; } @@ -273,6 +354,7 @@ static void free_argument(struct arg_node *arg) bool modulecmd_register_command(const char *domain, const char *identifier, MODULECMDFN entry_point, int argc, modulecmd_arg_type_t *argv) { + reset_error(); bool rval = false; spinlock_acquire(&modulecmd_lock); @@ -282,8 +364,8 @@ bool modulecmd_register_command(const char *domain, const char *identifier, { if (domain_has_command(dm, identifier)) { - MXS_ERROR("Command '%s' in domain '%s' was registered more than once.", - identifier, domain); + modulecmd_set_error("Command registered more than once: %s::%s", domain, identifier); + MXS_ERROR("Command registered more than once: %s::%s", domain, identifier); } else { @@ -305,6 +387,7 @@ bool modulecmd_register_command(const char *domain, const char *identifier, const MODULECMD* modulecmd_find_command(const char *domain, const char *identifier) { + reset_error(); MODULECMD *rval = NULL; spinlock_acquire(&modulecmd_lock); @@ -325,36 +408,37 @@ const MODULECMD* modulecmd_find_command(const char *domain, const char *identifi } spinlock_release(&modulecmd_lock); + + if (rval == NULL) + { + modulecmd_set_error("Command not found: %s::%s", domain, identifier); + } + return rval; } MODULECMD_ARG* modulecmd_arg_parse(const MODULECMD *cmd, int argc, const char **argv) { - int argc_min = 0; - - for (int i = 0; i < cmd->arg_count; i++) - { - if (MODULECMD_ARG_IS_REQUIRED(cmd->arg_types[i])) - { - argc_min++; - } - } + reset_error(); MODULECMD_ARG* arg = NULL; - if (argc >= argc_min && argc <= cmd->arg_count) + if (argc >= cmd->arg_count_min && argc <= cmd->arg_count_max) { - arg = modulecmd_arg_create(cmd->arg_count); + arg = modulecmd_arg_create(cmd->arg_count_max); bool error = false; if (arg) { - for (int i = 0; i < cmd->arg_count && i < argc; i++) + for (int i = 0; i < cmd->arg_count_max && i < argc; i++) { - if (!process_argument(cmd->arg_types[i], argv[i], &arg->argv[i])) + const char *err = ""; + + if (!process_argument(cmd->arg_types[i], argv[i], &arg->argv[i], &err)) { - MXS_ERROR("Failed to parse argument %d: %s", i + 1, argv[i] ? argv[i] : "NULL"); error = true; + modulecmd_set_error("Argument %d, %s: %s", i + 1, err, argv[i] ? argv[i] : "No argument given"); + break; } } @@ -365,6 +449,10 @@ MODULECMD_ARG* modulecmd_arg_parse(const MODULECMD *cmd, int argc, const char ** } } } + else + { + report_argc_mismatch(cmd, argc); + } return arg; } @@ -385,5 +473,38 @@ void modulecmd_arg_free(MODULECMD_ARG* arg) bool modulecmd_call_command(const MODULECMD *cmd, const MODULECMD_ARG *args) { - return cmd->func(args); + bool rval = false; + reset_error(); + + if (cmd->arg_count_min > 0 && args == NULL) + { + report_argc_mismatch(cmd, 0); + } + else + { + if (args == NULL) + { + args = &MODULECMD_NO_ARGUMENTS; + } + + rval = cmd->func(args); + } + + return rval; +} + +void modulecmd_set_error(const char *format, ...) +{ + prepare_error(); + + va_list list; + va_start(list, format); + vsnprintf(errbuf, MODULECMD_ERRBUF_SIZE, format, list); + va_end(list); +} + +const char* modulecmd_get_error() +{ + prepare_error(); + return errbuf; } diff --git a/server/core/test/testmodulecmd.c b/server/core/test/testmodulecmd.c index 7a1832aa8..be4a0a498 100644 --- a/server/core/test/testmodulecmd.c +++ b/server/core/test/testmodulecmd.c @@ -47,17 +47,21 @@ int test_arguments() const char *id = "test_arguments"; modulecmd_arg_type_t args1[] = {MODULECMD_ARG_STRING, MODULECMD_ARG_BOOLEAN}; + TEST(strlen(modulecmd_get_error()) == 0, "Error message should be empty"); + /** * Test command creation */ TEST(modulecmd_find_command(ns, id) == NULL, "The registered command should not yet be found"); + TEST(strlen(modulecmd_get_error()), "Error message should not be empty"); TEST(modulecmd_register_command(ns, id, test_fn, 2, args1), "Registering a command should succeed"); TEST(!modulecmd_register_command(ns, id, test_fn, 2, args1), "Registering the command a second time should fail"); + TEST(strlen(modulecmd_get_error()), "Error message should not be empty"); const MODULECMD *cmd = modulecmd_find_command(ns, id); TEST(cmd, "The registered command should be found"); @@ -67,13 +71,20 @@ int test_arguments() */ TEST(modulecmd_arg_parse(cmd, 0, NULL) == NULL, "Passing no arguments should fail"); + TEST(strlen(modulecmd_get_error()), "Error message should not be empty"); TEST(modulecmd_arg_parse(cmd, 1, params1) == NULL, "Passing one argument should fail"); + TEST(strlen(modulecmd_get_error()), "Error message should not be empty"); TEST(modulecmd_arg_parse(cmd, 3, params1) == NULL, "Passing three arguments should fail"); + TEST(strlen(modulecmd_get_error()), "Error message should not be empty"); TEST(modulecmd_arg_parse(cmd, 2, bad_params1) == NULL, "Passing bad arguments should fail"); + TEST(strlen(modulecmd_get_error()), "Error message should not be empty"); TEST(modulecmd_arg_parse(cmd, 2, bad_params2) == NULL, "Passing bad arguments should fail"); + TEST(strlen(modulecmd_get_error()), "Error message should not be empty"); TEST(modulecmd_arg_parse(cmd, 2, bad_params3) == NULL, "Passing bad arguments should fail"); + TEST(strlen(modulecmd_get_error()), "Error message should not be empty"); TEST(modulecmd_arg_parse(cmd, 2, bad_params4) == NULL, "Passing bad arguments should fail"); + TEST(strlen(modulecmd_get_error()), "Error message should not be empty"); /** * Test valid arguments @@ -81,6 +92,7 @@ int test_arguments() MODULECMD_ARG* alist = modulecmd_arg_parse(cmd, 2, params1); TEST(alist, "Arguments should be parsed"); + TEST(strlen(modulecmd_get_error()) == 0, "Error message should be empty"); TEST(modulecmd_call_command(cmd, alist), "Module call should be successful"); TEST(ok, "Function should receive right parameters"); @@ -95,7 +107,7 @@ int test_arguments() modulecmd_arg_free(alist); TEST((alist = modulecmd_arg_parse(cmd, 2, params2)), "Arguments should be parsed"); - + TEST(strlen(modulecmd_get_error()) == 0, "Error message should be empty"); TEST(modulecmd_call_command(cmd, alist), "Module call should be successful"); TEST(ok, "Function should receive right parameters"); @@ -105,11 +117,14 @@ int test_arguments() * Test valid but wrong arguments */ TEST((alist = modulecmd_arg_parse(cmd, 2, wrong_params1)), "Arguments should be parsed"); + TEST(strlen(modulecmd_get_error()) == 0, "Error message should be empty"); TEST(modulecmd_call_command(cmd, alist), "Module call should be successful"); + TEST(strlen(modulecmd_get_error()) == 0, "Error message should be empty"); TEST(!ok, "Function should receive wrong parameters"); modulecmd_arg_free(alist); TEST((alist = modulecmd_arg_parse(cmd, 2, wrong_params2)), "Arguments should be parsed"); + TEST(strlen(modulecmd_get_error()) == 0, "Error message should be empty"); TEST(modulecmd_call_command(cmd, alist), "Module call should be successful"); TEST(!ok, "Function should receive wrong parameters"); modulecmd_arg_free(alist); @@ -145,32 +160,79 @@ int test_optional_arguments() MODULECMD_ARG *arg = modulecmd_arg_parse(cmd, 2, params1); TEST(arg, "Parsing arguments should succeed"); + TEST(strlen(modulecmd_get_error()) == 0, "Error message should be empty"); + TEST(modulecmd_call_command(cmd, arg), "Module call should be successful"); + TEST(strlen(modulecmd_get_error()) == 0, "Error message should be empty"); modulecmd_arg_free(arg); arg = modulecmd_arg_parse(cmd, 2, params2); TEST(arg, "Parsing arguments should succeed"); + TEST(strlen(modulecmd_get_error()) == 0, "Error message should be empty"); + TEST(modulecmd_call_command(cmd, arg), "Module call should be successful"); + TEST(strlen(modulecmd_get_error()) == 0, "Error message should be empty"); modulecmd_arg_free(arg); arg = modulecmd_arg_parse(cmd, 2, params3); TEST(arg, "Parsing arguments should succeed"); + TEST(strlen(modulecmd_get_error()) == 0, "Error message should be empty"); + TEST(modulecmd_call_command(cmd, arg), "Module call should be successful"); + TEST(strlen(modulecmd_get_error()) == 0, "Error message should be empty"); modulecmd_arg_free(arg); arg = modulecmd_arg_parse(cmd, 2, params4); TEST(arg, "Parsing arguments should succeed"); + TEST(strlen(modulecmd_get_error()) == 0, "Error message should be empty"); + TEST(modulecmd_call_command(cmd, arg), "Module call should be successful"); + TEST(strlen(modulecmd_get_error()) == 0, "Error message should be empty"); modulecmd_arg_free(arg); arg = modulecmd_arg_parse(cmd, 1, params1); TEST(arg, "Parsing arguments should succeed"); + TEST(strlen(modulecmd_get_error()) == 0, "Error message should be empty"); + TEST(modulecmd_call_command(cmd, arg), "Module call should be successful"); + TEST(strlen(modulecmd_get_error()) == 0, "Error message should be empty"); modulecmd_arg_free(arg); arg = modulecmd_arg_parse(cmd, 1, params2); TEST(arg, "Parsing arguments should succeed"); + TEST(strlen(modulecmd_get_error()) == 0, "Error message should be empty"); + TEST(modulecmd_call_command(cmd, arg), "Module call should be successful"); + TEST(strlen(modulecmd_get_error()) == 0, "Error message should be empty"); modulecmd_arg_free(arg); arg = modulecmd_arg_parse(cmd, 0, params1); TEST(arg, "Parsing arguments should succeed"); + TEST(strlen(modulecmd_get_error()) == 0, "Error message should be empty"); + TEST(modulecmd_call_command(cmd, arg), "Module call should be successful"); + TEST(strlen(modulecmd_get_error()) == 0, "Error message should be empty"); modulecmd_arg_free(arg); + TEST(modulecmd_call_command(cmd, NULL), "Module call should be successful"); + TEST(strlen(modulecmd_get_error()) == 0, "Error message should be empty"); + + return 0; +} + +bool test_fn3(const MODULECMD_ARG *arg) +{ + modulecmd_set_error("Something went wrong!"); + return false; +} + +int test_module_errors() +{ + const char *ns = "test_module_errors"; + const char *id = "test_module_errors"; + + TEST(modulecmd_register_command(ns, id, test_fn3, 0, NULL), + "Registering a command should succeed"); + + const MODULECMD *cmd = modulecmd_find_command(ns, id); + TEST(cmd, "The registered command should be found"); + + TEST(!modulecmd_call_command(cmd, NULL), "Module call should fail"); + TEST(strlen(modulecmd_get_error()), "Error message should not be empty"); + return 0; } @@ -180,6 +242,7 @@ int main(int argc, char **argv) rc += test_arguments(); rc += test_optional_arguments(); + rc += test_module_errors(); return rc; }