MXS-929: Add errors to the modulecmd system

The modules can now return human-readable error messages to the caller of
the function. The internals of the modulecmd system also use this to
return errors to the users.

The error messages can be retrieved with a common error function which
should make it easy to use in various clients.
This commit is contained in:
Markus Makela 2016-11-19 01:43:16 +02:00
parent 4603e71987
commit 4a142b1ca9
4 changed files with 233 additions and 30 deletions

View File

@ -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

View File

@ -2010,7 +2010,7 @@ config_truth_value(char *str)
{
return 0;
}
MXS_ERROR("Not a boolean value: %s", str);
return -1;
}

View File

@ -14,8 +14,18 @@
#include <maxscale/alloc.h>
#include <maxscale/config.h>
#include <maxscale/modulecmd.h>
#include <maxscale/platform.h>
#include <maxscale/spinlock.h>
/** 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;
}

View File

@ -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;
}