Re: [Qemu-devel] [PATCH] RFC: options parse in vl.c should be moduled

["Daniel P. Berrange" <berrange@redhat.com>]

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 <liwp@linux.vnet.ibm.com>
> ---
>  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
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org       -o-       http://live.gnome.org/gtk-vnc :|