[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH] RFC: options parse in vl.c should be moduled
From: |
Wanpeng Li |
Subject: |
Re: [Qemu-devel] [PATCH] RFC: options parse in vl.c should be moduled |
Date: |
Fri, 30 Mar 2012 21:25:47 +0800 |
User-agent: |
Mutt/1.5.21 (2010-09-15) |
On Fri, Mar 30, 2012 at 01:53:14PM +0100, Daniel P. Berrange wrote:
>On Fri, Mar 30, 2012 at 08:36:43PM +0800, Wanpeng Li wrote:
>> Consider of the options parse process in main function of vl.c is too
>> long.It should be module into single function to clear ideas, strengthen
>> the source code management, and increase code readability.So I module the
>> process of options parse as function options_parse, and expose some variables
>> in order to not influence command-line invocations.
>>
>> Signed-off-by: Wanpeng Li <address@hidden>
>> ---
>> vl.c | 159
>> ++++++++++++++++++++++++++++++++++-------------------------------
>> 1 files changed, 83 insertions(+), 76 deletions(-)
>>
>> diff --git a/vl.c b/vl.c
>> index 0fccf50..fa4d0a9 100644
>> --- a/vl.c
>> +++ b/vl.c
>> @@ -2251,84 +2251,40 @@ int qemu_init_main_loop(void)
>> return main_loop_init();
>> }
>>
>> -int main(int argc, char **argv, char **envp)
>> -{
>> - int i;
>> - int snapshot, linux_boot;
>> - const char *icount_option = NULL;
>> - const char *initrd_filename;
>> - const char *kernel_filename, *kernel_cmdline;
>> - char boot_devices[33] = "cad"; /* default to HD->floppy->CD-ROM */
>> - DisplayState *ds;
>> - DisplayChangeListener *dcl;
>> - int cyls, heads, secs, translation;
>> - QemuOpts *hda_opts = NULL, *opts, *machine_opts;
>> - QemuOptsList *olist;
>> - int optind;
>> - const char *optarg;
>> - const char *loadvm = NULL;
>> - QEMUMachine *machine;
>> - const char *cpu_model;
>> - const char *vga_model = NULL;
>> - const char *pid_file = NULL;
>> - const char *incoming = NULL;
>> +int snapshot, linux_boot;
>> +const char *icount_option;
>> +const char *initrd_filename;
>> +const char *kernel_filename, *kernel_cmdline;
>> +char boot_devices[33] = "cad"; /* default to HD->floppy->CD-ROM */
>> +DisplayState *ds;
>> +DisplayChangeListener *dcl;
>> +int cyls, heads, secs, translation;
>> +QemuOpts *hda_opts , *opts, *machine_opts;
>> +QemuOptsList *olist;
>> +int optind;
>> +const char *loadvm;
>> +QEMUMachine *machine;
>> +const char *cpu_model;
>> +const char *vga_model;
>> +const char *pid_file;
>> +const char *incoming;
>> #ifdef CONFIG_VNC
>> - int show_vnc_port = 0;
>> +int show_vnc_port;
>
>[snip]
>
>> +int defconfig = 1;
>> +const char *log_mask;
>> +const char *log_file;
>> +GMemVTable mem_trace = {
>> + .malloc = malloc_and_trace,
>> + .realloc = realloc_and_trace,
>> + .free = free_and_trace,
>> +};
>> +const char *trace_events;
>> +const char *trace_file;
>>
>> +static void options_parse(int argc, char **argv)
>> +{
>
>While code modularization is a worthy goal, I don't think this patch is
>really an improvement. QEMU already has far too many adhoc global variables,
>without adding another 30 or more. The resulting code isn't even simplified
>or more readable IMHO, it is merely different.
>
>Daniel
There are about 856 lines of codes handle options parse in main function.
It is ugly and reduce readability. So I module these codes to a
single function called "options_parse".Since there are amounts of
command_line parameters which lead to must transfer many parameters to function
options_parse, so I expose some variables to global in order to handler
this issue.
Regards,
Wanpeng Li
--
LTC China, IBM, Shanghai