[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 4/9] QMP: Second half of the new argument checki
From: |
Markus Armbruster |
Subject: |
Re: [Qemu-devel] [PATCH 4/9] QMP: Second half of the new argument checking code |
Date: |
Wed, 02 Jun 2010 16:41:43 +0200 |
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/23.1 (gnu/linux) |
Luiz Capitulino <address@hidden> writes:
> On Wed, 02 Jun 2010 09:31:24 +0200
> Markus Armbruster <address@hidden> wrote:
>
>> Luiz Capitulino <address@hidden> 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 <address@hidden>
>> > ---
>> > 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.
>
> Yes, there're two args_type we don't handle in QMP because we don't have
> any of those handlers converted: '/' and '.'.
>
> I think it's unlikely to get them converted in this form, so the current
> implementation contains dead-code. I explained this in one commit log, but
> maybe it's the wrong place.
I read that commit message after I sent my comment :)
The commit message is fine; I'd like a comment in the code in addition,
not instead of the commit message.
[...]
- Re: [Qemu-devel] [PATCH 3/9] QMP: First half of the new argument checking code, (continued)
[Qemu-devel] [PATCH 2/9] Monitor: handle optional '-' arg as a bool, Luiz Capitulino, 2010/06/01
[Qemu-devel] [PATCH 5/9] QMP: Drop old client argument checker, Luiz Capitulino, 2010/06/01
[Qemu-devel] [PATCH 6/9] QMP: check_opts(): Minor cleanup, Luiz Capitulino, 2010/06/01
[Qemu-devel] [PATCH 4/9] QMP: Second half of the new argument checking code, Luiz Capitulino, 2010/06/01
- Re: [Qemu-devel] [PATCH 4/9] QMP: Second half of the new argument checking code, Markus Armbruster, 2010/06/02
- [Qemu-devel] Handling the O-type, Luiz Capitulino, 2010/06/18
- Re: [Qemu-devel] Handling the O-type, Markus Armbruster, 2010/06/21
- Re: [Qemu-devel] Handling the O-type, Luiz Capitulino, 2010/06/21
- Re: [Qemu-devel] Handling the O-type, Markus Armbruster, 2010/06/21
[Qemu-devel] [PATCH 7/9] QError: Introduce QERR_QMP_BAD_INPUT_OBJECT_MEMBER, Luiz Capitulino, 2010/06/01
[Qemu-devel] [PATCH 8/9] QMP: Introduce qmp_check_input_obj(), Luiz Capitulino, 2010/06/01