Luiz Capitulino <lcapitul...@redhat.com> writes:
> This commit introduces check_client_args_type(), which is
> called by qmp_check_client_args() and complements the
> previous commit.
>
> Now the new client's argument checker code is capable of
> doing type checking and detecting unknown arguments.
>
> It works this way: we iterate over the client's arguments
> qdict and for each argument we check if it exists and if
> its type is correct.
>
> Signed-off-by: Luiz Capitulino <lcapitul...@redhat.com>
> ---
> monitor.c | 77
> ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++-
> 1 files changed, 76 insertions(+), 1 deletions(-)
>
> diff --git a/monitor.c b/monitor.c
> index 47a0da8..14790e6 100644
> --- a/monitor.c
> +++ b/monitor.c
> @@ -4266,6 +4266,75 @@ typedef struct QMPArgCheckRes {
> } QMPArgCheckRes;
>
> /*
> + * Check if client's argument exists and type is correct
> + */
> +static void check_client_args_type(const char *client_arg_name,
> + QObject *client_arg, void *opaque)
> +{
> + QObject *obj;
> + QString *arg_type;
> + QMPArgCheckRes *res = opaque;
> +
> + if (res->result < 0) {
> + /* report only the first error */
> + return;
> + }
> +
> + obj = qdict_get(res->qdict, client_arg_name);
> + if (!obj) {
> + /* client arg doesn't exist */
> + res->result = -1;
> + qerror_report(QERR_INVALID_PARAMETER, client_arg_name);
> + return;
> + }
> +
> + 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");
> + res->result = -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");
> + res->result = -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");
> + res->result = -1;
> + }
> + break;
> + case 'b':
> + case '-':
> + if (qobject_type(client_arg) != QTYPE_QBOOL) {
> + qerror_report(QERR_INVALID_PARAMETER_TYPE, client_arg_name,
> "bool");
> + res->result = -1;
> + }
> + break;
> + case 'O':
> + /* Not checked here */
> + break;
What about case '/'? I guess it doesn't make much sense for QMP, but
the old checker handles it. If we drop it from QMP, we should document
the restriction in the source.
> + default:
> + abort();
> + }
> +}
> +
> +/*
> * Check if client passed all mandatory args
> */
> static void check_mandatory_args(const char *cmd_arg_name,
> @@ -4344,6 +4413,9 @@ out:
> * Client argument checking rules:
> *
> * 1. Client must provide all mandatory arguments
> + * 2. Each argument provided by the client must be valid
> + * 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)
> {
> @@ -4355,7 +4427,10 @@ static int qmp_check_client_args(const mon_cmd_t *cmd,
> QDict *client_args)
> res.qdict = client_args;
> qdict_iter(cmd_args, check_mandatory_args, &res);
>
> - /* TODO: Check client args type */
> + if (!res.result && !res.skip) {
> + res.qdict = cmd_args;
> + qdict_iter(client_args, check_client_args_type, &res);
> + }
What if we have both an O-type argument and other arguments? Then the
'O' makes check_client_args_type() set res.skip, and we duly skip
checking the other arguments here.
Again, the iterator makes for tortuous code.
>
> QDECREF(cmd_args);
> return res.result;
