From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mailman by lists.gnu.org with tmda-scanned (Exim 4.43) id 1Mja5b-00077N-4P for qemu-devel@nongnu.org; Fri, 04 Sep 2009 10:48:35 -0400 Received: from exim by lists.gnu.org with spam-scanned (Exim 4.43) id 1Mja5V-00072z-2X for qemu-devel@nongnu.org; Fri, 04 Sep 2009 10:48:34 -0400 Received: from [199.232.76.173] (port=37265 helo=monty-python.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1Mja5U-00072T-NJ for qemu-devel@nongnu.org; Fri, 04 Sep 2009 10:48:28 -0400 Received: from mx1.redhat.com ([209.132.183.28]:1025) by monty-python.gnu.org with esmtp (Exim 4.60) (envelope-from ) id 1Mja5U-00081X-6S for qemu-devel@nongnu.org; Fri, 04 Sep 2009 10:48:28 -0400 Date: Fri, 4 Sep 2009 11:48:11 -0300 From: Luiz Capitulino Subject: Re: [Qemu-devel] [PATCH 06/29] monitor: New format for handlers argument types Message-ID: <20090904114811.39da3ff8@doriath> In-Reply-To: <4AA11EDF.1000609@codemonkey.ws> References: <1250723280-3509-1-git-send-email-lcapitulino@redhat.com> <1250723280-3509-7-git-send-email-lcapitulino@redhat.com> <87skfhi3nm.fsf@pike.pond.sub.org> <20090824140026.4636828e@doriath> <4AA11EDF.1000609@codemonkey.ws> Mime-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Anthony Liguori Cc: avi@redhat.com, Markus Armbruster , qemu-devel@nongnu.org On Fri, 04 Sep 2009 09:06:23 -0500 Anthony Liguori wrote: > Luiz Capitulino wrote: > > On Mon, 24 Aug 2009 18:21:17 +0200 > > Markus Armbruster wrote: > > > > > >> Luiz Capitulino writes: > >> > >> > >>> Current handlers argument types, as defined in qemu-monitor.hx file, > >>> are a sequence of chars where each one represents one argument type > >>> of the command handler. The number of chars is also used to know how > >>> many arguments a given handler accepts. > >>> > >>> This commit defines a new format, which makes mandatory the use of > >>> a name for each argument. > >>> > >>> For example, do_eject() command handler is currently defined as: > >>> > >>> { "eject", "-fB", do_eject, ... } > >>> > >>> With the new format it becomes: > >>> > >>> { "eject", "force:-f,filename:B", do_eject, ... } > >>> > >>> This way the Monitor will be capable of setting up a dictionary, using > >>> each argument's name as the key and the argument itself as the value. > >>> > >>> This commit also adds two new functions: key_get_info() and > >>> next_arg_type(), both are used to parse the new format. > >>> > >>> Currently key_get_info() consumes the 'key' part of the new format and > >>> discards it, this way the current parsing code is not affected by this > >>> change. > >>> > >>> Signed-off-by: Luiz Capitulino > >>> > >> Encoding the parameter list in a single args_type made perfect sense > >> when a parameter was encoded in one or two characters. But having > >> syntax and a parser... I don't know. Switch to an array of parameter > >> descriptions that don't need to be parsed? > >> > > > > I don't see how to do this w/o parsing, but anyway I think we should > > be practical here and use what already exists. > > > I agree with Markus. I think we can start with this approach but I'd > like to see it changed in the near future. Instead of having: > > { "eject", "force:-f,filename:B", do_eject, ... } > > I would expect something like: > > { .name = "eject", > .args = (MonitorArgs[]){ > { .name = "force", > .short = 'f', > .type = MON_FLAG }, > {}}, > .func = do_eject, > } > > Macros can be used to simplify things but I think this is the general > idea that Markus was suggesting. It's a good improvement indeed, couldn't see at first, sorry about that.