From mboxrd@z Thu Jan  1 00:00:00 1970
Received: from mailman by lists.gnu.org with tmda-scanned (Exim 4.43)
	id 1JzXLF-0001Ym-UQ
	for qemu-devel@nongnu.org; Fri, 23 May 2008 09:29:53 -0400
Received: from exim by lists.gnu.org with spam-scanned (Exim 4.43)
	id 1JzXLE-0001Ya-FC
	for qemu-devel@nongnu.org; Fri, 23 May 2008 09:29:53 -0400
Received: from [199.232.76.173] (port=43972 helo=monty-python.gnu.org)
	by lists.gnu.org with esmtp (Exim 4.43) id 1JzXLE-0001YX-8h
	for qemu-devel@nongnu.org; Fri, 23 May 2008 09:29:52 -0400
Received: from rn-out-0910.google.com ([64.233.170.185]:41156)
	by monty-python.gnu.org with esmtp (Exim 4.60)
	(envelope-from <glommer@gmail.com>) id 1JzXLE-0000Y3-Bk
	for qemu-devel@nongnu.org; Fri, 23 May 2008 09:29:52 -0400
Received: by rn-out-0910.google.com with SMTP id j71so486831rne.8
	for <qemu-devel@nongnu.org>; Fri, 23 May 2008 06:29:50 -0700 (PDT)
Message-ID: <5d6222a80805230629u301fcc1cu7bdaeb25bc4dfb67@mail.gmail.com>
Date: Fri, 23 May 2008 10:29:49 -0300
From: "Glauber Costa" <glommer@gmail.com>
Subject: Re: [Qemu-devel] Re: [PATCH 1/3] Modular command line options
In-Reply-To: <48356BC4.3090805@web.de>
MIME-Version: 1.0
Content-Type: text/plain; charset=ISO-8859-1
Content-Transfer-Encoding: 7bit
Content-Disposition: inline
References: <48336413.2060909@web.de>
	<18485.18619.634431.859466@mariner.uk.xensource.com>
	<48356BC4.3090805@web.de>
Reply-To: qemu-devel@nongnu.org
List-Id: qemu-devel.nongnu.org
List-Unsubscribe: <http://lists.nongnu.org/mailman/listinfo/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=unsubscribe>
List-Archive: <http://lists.gnu.org/pipermail/qemu-devel>
List-Post: <mailto:qemu-devel@nongnu.org>
List-Help: <mailto:qemu-devel-request@nongnu.org?subject=help>
List-Subscribe: <http://lists.nongnu.org/mailman/listinfo/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=subscribe>
To: qemu-devel@nongnu.org

On Thu, May 22, 2008 at 9:49 AM, Jan Kiszka <jan.kiszka@web.de> wrote:
> Ian Jackson wrote:
>> Jan Kiszka writes ("[Qemu-devel] [PATCH 1/3] Modular command line options"):
>>> Following up on my earlier proposal to introduce per-machine command
>>> line options, this version provides a more generic approach. It should
>>> also be usable for scenarios like per-arch or per-accelerator.
>>
>> I approve of splitting the code up like this, and having a
>> table-driven parsing arrangement.  But ideally we could get rid of
>> `index' and the giant switch() statements too.  Something more like
>>
>>   typedef void QEMUOptionParser(struct QEMUOption *option, const char *optarg);
>>
>>   typedef struct QEMUOption {
>>       const char *name, *helpstring;
>>       QEMUOptionParser handler;
>>       int flags;
>
> Ack. This just enforces a bit more effort to convert the existing
> opts... :->
>
>>       int int_for_handler;
>>       void *void_for_handler;
>
> I don't think there is an need for both. A plain
>
>        void *parser_opaque;
>
> should suffice as the user can perfectly typecast the void to int.
>
>>   } QEMUOption;
>>
>>   qemu_register_option_set(const QEMUOption *options);
>
> Here I would then suggest
>
>        qemu_register_option_set(const char *set_name,
>                                 const QEMUOption *options);
>
> to save the chance for visually grouping options.

I don't follow the need for the set_name parameter. This should be in
the OptionSet structure.
You mean, joining two groups if they are registered with the same
name? This can be done by enforcing them to have
the same "name" parameter in the OptionSet structure. Am I failing to
understand anything here?

>
>
> Good idea. Then the structure should look like this:
>
> struct QEMUOptionSet {
>        const char *name;
          ^^^^^^^^^^^^


-- 
Glauber Costa.
"Free as in Freedom"
http://glommer.net

"The less confident you are, the more serious you have to act."