From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:47420) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1SDeAs-0002b9-OR for qemu-devel@nongnu.org; Fri, 30 Mar 2012 11:55:44 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1SDeAl-0002ll-J3 for qemu-devel@nongnu.org; Fri, 30 Mar 2012 11:55:38 -0400 Received: from mail-ob0-f173.google.com ([209.85.214.173]:42865) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1SDeAl-0002lY-AJ for qemu-devel@nongnu.org; Fri, 30 Mar 2012 11:55:31 -0400 Received: by obbwd20 with SMTP id wd20so1260752obb.4 for ; Fri, 30 Mar 2012 08:55:29 -0700 (PDT) Sender: fluxion Date: Fri, 30 Mar 2012 10:55:24 -0500 From: Michael Roth Message-ID: <20120330155524.GC22887@illuin> References: <1333111003-28556-1-git-send-email-liwp@linux.vnet.ibm.com> <20120330125313.GH10487@redhat.com> <4f75b5f3.2235b60a.302d.ffffa586SMTPIN_ADDED@mx.google.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <4f75b5f3.2235b60a.302d.ffffa586SMTPIN_ADDED@mx.google.com> Subject: Re: [Qemu-devel] [PATCH] RFC: options parse in vl.c should be moduled List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Wanpeng Li Cc: Anthony Liguori , Gavin Shan , Stefan Hajnoczi , qemu-devel , =?iso-8859-1?Q?AndreasF=E4rber?= On Fri, Mar 30, 2012 at 09:25:47PM +0800, Wanpeng Li wrote: > 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. Anthony's aforementioned patches avoid the new globals by moving them into a single global configuration struct. Keeps things a little tidier and makes is easier to recognize when code is accessing a global config option. > > Regards, > Wanpeng Li > > -- > LTC China, IBM, Shanghai > >