Luiz Capitulino <lcapitul...@redhat.com> writes:
> This commit introduces the second (and last) part of QMP's new
> argument checker.
>
> The job is done by check_client_args_type(), it iterates over
> the client's argument qdict and for for each argument it checks
> if it exists and if its type is valid.
>
> It's important to observe the following changes from the existing
> argument checker:
>
> - If the handler accepts an O-type argument, unknown arguments
> are passed down to it. It's up to O-type handlers to validate
> their arguments
>
> - Boolean types (eg. 'b' and '-') don't accept integers anymore,
> only json-bool
>
> - Argument types '/' and '.' are currently unsupported under QMP,
> thus they're not handled
>
> Signed-off-by: Luiz Capitulino <lcapitul...@redhat.com>
> ---
> monitor.c | 100
> ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++-
> 1 files changed, 99 insertions(+), 1 deletions(-)
>
> diff --git a/monitor.c b/monitor.c
> index b4fe5ba..8d074c2 100644
> --- a/monitor.c
> +++ b/monitor.c
> @@ -4139,6 +4139,101 @@ static int invalid_qmp_mode(const Monitor *mon, const
> char *cmd_name)
> }
>
> /*
> + * Argument validation rules:
> + *
> + * 1. The argument must exist in cmd_args qdict
> + * 2. The argument type must be the expected one
> + *
> + * Special case: If the argument doesn't exist in cmd_args and
> + * the QMP_CHECKER_OTYPE flag is set, then the
> + * argument is considered an O-type one and the
> + * checking is skipped for it.
> + */
> +static int check_client_args_type(const QDict *client_args,
> + const QDict *cmd_args, int flags)
> +{
> + const QDictEntry *ent;
> +
> + for (ent = qdict_first(client_args); ent;ent =
> qdict_next(client_args,ent)){
> + QObject *obj;
> + QString *arg_type;
> + const QObject *client_arg = qdict_entry_value(ent);
> + const char *client_arg_name = qdict_entry_key(ent);
> +
> + obj = qdict_get(cmd_args, client_arg_name);
> + if (!obj) {
> + if (flags & QMP_CHECKER_OTYPE) {
> + /*
> + * This handler accepts O-type arguments, it's up to it to
> + * check for unknowns and validate its type.
> + */
> + continue;
> + }
> + /* client arg doesn't exist */
> + qerror_report(QERR_INVALID_PARAMETER, client_arg_name);
> + return -1;
> + }
> +
> + arg_type = qobject_to_qstring(obj);
> + assert(arg_type != NULL);
> +
> + /* check if argument's type is correct */
> + switch (qstring_get_str(arg_type)[0]) {
> + case 'F':
> + case 'B':
> + case 's':
> + if (qobject_type(client_arg) != QTYPE_QSTRING) {
> + qerror_report(QERR_INVALID_PARAMETER_TYPE, client_arg_name,
> + "string");
> + return -1;
> + }
> + break;
> + case 'i':
> + case 'l':
> + case 'M':
> + if (qobject_type(client_arg) != QTYPE_QINT) {
> + qerror_report(QERR_INVALID_PARAMETER_TYPE, client_arg_name,
> + "int");
> + return -1;
> + }
> + break;
> + case 'f':
> + case 'T':
> + if (qobject_type(client_arg) != QTYPE_QINT &&
> + qobject_type(client_arg) != QTYPE_QFLOAT) {
> + qerror_report(QERR_INVALID_PARAMETER_TYPE, client_arg_name,
> + "number");
> + return -1;
> + }
> + break;
> + case 'b':
> + case '-':
> + if (qobject_type(client_arg) != QTYPE_QBOOL) {
> + qerror_report(QERR_INVALID_PARAMETER_TYPE, client_arg_name,
> + "bool");
> + return -1;
> + }
> + break;
> + case 'O':
> + /* XXX: this argument has the same name of the O-type defined in
> + in qemu-monitor.hx. This is not allowed, right? */
No, it's actually fine.
Consider device_add. Its args_type is "device:O". Nevertheless, it's
pefectly okay for a qdev to have a property named "device".
> + qerror_report(QERR_INVALID_PARAMETER, client_arg_name);
> + return -1;
Instead:
assert(flags & QMP_CHECKER_OTYPE);
continue;
Not sure I like the name QMP_CHECKER_OTYPE. The way it's used, it means
"in addition to checking declared arguments, accept undeclared arguments
without checking them (somebody else will check)". 'O-type' is merely
something that triggers that flag. Happens to be the only way right
now.
QMP_CHECKER_ACCEPT_MORE_ARGS?
> + case '/':
> + case '.':
> + /*
> + * These types are not supported by QMP and thus are not
> + * handled here. Fall through.
> + */
> + default:
> + abort();
> + }
> + }
> +
> + return 0;
> +}
> +
> +/*
> * - Check if the client has passed all mandatory args
> * - Set special flags for argument validation
> */
> @@ -4215,6 +4310,9 @@ out:
> * Client argument checking rules:
> *
> * 1. Client must provide all mandatory arguments
> + * 2. Each argument provided by the client must be expected
> + * 3. Each argument provided by the client must have the type expected
> + * by the command
> */
> static int qmp_check_client_args(const mon_cmd_t *cmd, QDict *client_args)
> {
> @@ -4229,7 +4327,7 @@ static int qmp_check_client_args(const mon_cmd_t *cmd,
> QDict *client_args)
> goto out;
> }
>
> - /* TODO: Check client args type */
> + err = check_client_args_type(client_args, cmd_args, flags);
>
> out:
> QDECREF(cmd_args);
