From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from [140.186.70.92] (port=59122 helo=eggs.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1OJiQF-0000hA-Aj for qemu-devel@nongnu.org; Wed, 02 Jun 2010 03:31:32 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.69) (envelope-from ) id 1OJiQC-0004YO-RC for qemu-devel@nongnu.org; Wed, 02 Jun 2010 03:31:31 -0400 Received: from mx1.redhat.com ([209.132.183.28]:11180) by eggs.gnu.org with esmtp (Exim 4.69) (envelope-from ) id 1OJiQC-0004YI-Jw for qemu-devel@nongnu.org; Wed, 02 Jun 2010 03:31:28 -0400 From: Markus Armbruster Subject: Re: [Qemu-devel] [PATCH 4/9] QMP: Second half of the new argument checking code References: <1275424897-32253-1-git-send-email-lcapitulino@redhat.com> <1275424897-32253-5-git-send-email-lcapitulino@redhat.com> Date: Wed, 02 Jun 2010 09:31:24 +0200 In-Reply-To: <1275424897-32253-5-git-send-email-lcapitulino@redhat.com> (Luiz Capitulino's message of "Tue, 1 Jun 2010 17:41:32 -0300") Message-ID: MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Luiz Capitulino Cc: jan.kiszka@siemens.com, qemu-devel@nongnu.org Luiz Capitulino 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 > --- > 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;