From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:32825) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1SDbqq-0004MB-4f for qemu-devel@nongnu.org; Fri, 30 Mar 2012 09:26:52 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1SDbql-0000i9-28 for qemu-devel@nongnu.org; Fri, 30 Mar 2012 09:26:47 -0400 Received: from e23smtp03.au.ibm.com ([202.81.31.145]:60034) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1SDbqk-0000hR-GJ for qemu-devel@nongnu.org; Fri, 30 Mar 2012 09:26:42 -0400 Received: from /spool/local by e23smtp03.au.ibm.com with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted for from ; Fri, 30 Mar 2012 13:17:51 +1000 Received: from d23av02.au.ibm.com (d23av02.au.ibm.com [9.190.235.138]) by d23relay05.au.ibm.com (8.13.8/8.13.8/NCO v10.0) with ESMTP id q2UDJcib2003150 for ; Sat, 31 Mar 2012 00:19:42 +1100 Received: from d23av02.au.ibm.com (loopback [127.0.0.1]) by d23av02.au.ibm.com (8.14.4/8.13.1/NCO v10.0 AVout) with ESMTP id q2UDPmjc028436 for ; Sat, 31 Mar 2012 00:25:49 +1100 Date: Fri, 30 Mar 2012 21:25:47 +0800 From: Wanpeng Li Message-ID: <20120330132547.GA1039@liwp@linux.vnet.ibm.com> References: <1333111003-28556-1-git-send-email-liwp@linux.vnet.ibm.com> <20120330125313.GH10487@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20120330125313.GH10487@redhat.com> Subject: Re: [Qemu-devel] [PATCH] RFC: options parse in vl.c should be moduled Reply-To: Wanpeng Li List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: "Daniel P. Berrange" Cc: qemu-devel , Stefan Hajnoczi , Anthony Liguori , Gavin Shan , =?iso-8859-1?Q?AndreasF=E4rber?= 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 >> --- >> 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