From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from [140.186.70.92] (port=57742 helo=eggs.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1OJhv3-0004Cw-6p for qemu-devel@nongnu.org; Wed, 02 Jun 2010 02:59:22 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.69) (envelope-from ) id 1OJhv1-0008R4-RX for qemu-devel@nongnu.org; Wed, 02 Jun 2010 02:59:17 -0400 Received: from mx1.redhat.com ([209.132.183.28]:20701) by eggs.gnu.org with esmtp (Exim 4.69) (envelope-from ) id 1OJhv1-0008Qs-KU for qemu-devel@nongnu.org; Wed, 02 Jun 2010 02:59:15 -0400 From: Markus Armbruster Subject: Re: [Qemu-devel] [PATCH 3/9] QMP: First half of the new argument checking code References: <1275424897-32253-1-git-send-email-lcapitulino@redhat.com> <1275424897-32253-4-git-send-email-lcapitulino@redhat.com> Date: Wed, 02 Jun 2010 08:59:11 +0200 In-Reply-To: <1275424897-32253-4-git-send-email-lcapitulino@redhat.com> (Luiz Capitulino's message of "Tue, 1 Jun 2010 17:41:31 -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 the first half of qmp_check_client_args(), > which is the new client argument checker. > > It's introduced on top of the existing code, so that there are > no regressions during the transition. > > It works this way: the command's args_type field (from > qemu-monitor.hx) is transformed into a qdict. Then we iterate > over it checking whether each mandatory argument has been > provided by the client. > > All comunication between the iterator and its caller is done > via the new QMPArgCheckRes type. > > Next commit adds the second half of this work: type checking > and invalid argument detection. > > Signed-off-by: Luiz Capitulino > --- > monitor.c | 107 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ > 1 files changed, 107 insertions(+), 0 deletions(-) > > diff --git a/monitor.c b/monitor.c > index bc3cc18..47a0da8 100644 > --- a/monitor.c > +++ b/monitor.c > @@ -4259,6 +4259,108 @@ static int invalid_qmp_mode(const Monitor *mon, const char *cmd_name) > return (qmp_cmd_mode(mon) ? is_cap : !is_cap); > } > > +typedef struct QMPArgCheckRes { > + int result; > + int skip; skip is write-only in this patch. > + QDict *qdict; > +} QMPArgCheckRes; > + > +/* > + * Check if client passed all mandatory args > + */ > +static void check_mandatory_args(const char *cmd_arg_name, > + QObject *obj, void *opaque) > +{ > + QString *type; > + QMPArgCheckRes *res = opaque; > + > + if (res->result < 0) { > + /* report only the first error */ > + return; > + } This is a sign that the iterator needs a way to break the loop. > + > + type = qobject_to_qstring(obj); > + assert(type != NULL); > + > + if (qstring_get_str(type)[0] == 'O') { > + QemuOptsList *opts_list = qemu_find_opts(cmd_arg_name); > + assert(opts_list); > + res->result = check_opts(opts_list, res->qdict); I doubt this is the right place for calling check_opts. Right now, it's only implemented for QemuOptsList with empty desc. A more complete version is in my tree (blockdev needs it). Looks like this: static int check_opts(QemuOptsList *opts_list, QDict *args) { QemuOptDesc *desc; CmdArgs cmd_args; for (desc = opts_list->desc; desc->name; desc++) { cmd_args_init(&cmd_args); cmd_args.optional = 1; switch (desc->type) { case QEMU_OPT_STRING: cmd_args.type = 's'; break; case QEMU_OPT_BOOL: cmd_args.type = '-'; break; case QEMU_OPT_NUMBER: case QEMU_OPT_SIZE: cmd_args.type = 'l'; break; } qstring_append(cmd_args.name, desc->name); if (check_arg(&cmd_args, args) < 0) { QDECREF(cmd_args.name); return -1; } QDECREF(cmd_args.name); } return 0; } > + res->skip = 1; > + } else if (qstring_get_str(type)[0] != '-' && > + qstring_get_str(type)[1] != '?' && > + !qdict_haskey(res->qdict, cmd_arg_name)) { > + res->result = -1; > + qerror_report(QERR_MISSING_PARAMETER, cmd_arg_name); > + } > +} [...]