Re: [PATCH] tools: Refactor code in libxl/xl_cmdimpl.c

[Wei Liu <wei.liu2@citrix.com>]

On Mon, Oct 20, 2014 at 03:01:28AM +0300, Alexandra Sandulescu wrote:
> This patch removes duplicate code in /tools/libxl/xl_cmdimpl.c by
> adding set_device_data function. This function parses configuration
> data and adds the information into libxl_device_nic struct. It is
> called in both main_networkattach and parse_config_data functions
> to replace duplicate code.
> ---
>  tools/libxl/xl_cmdimpl.c | 160 +++++++++++++++++------------------------------
>  1 file changed, 57 insertions(+), 103 deletions(-)
> 
> diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c
> index 988ee28..449aa91 100644
> --- a/tools/libxl/xl_cmdimpl.c
> +++ b/tools/libxl/xl_cmdimpl.c
> @@ -823,6 +823,8 @@ static void parse_vcpu_affinity(libxl_domain_build_info *b_info,
>      }
>  }
>  
> +static int set_device_data(libxl_device_nic *nic, XLU_Config **config, char *token);
> +

Since this function operates on libxl_device_nic, it should be named
accordingly.

>  static void parse_config_data(const char *config_source,
>                                const char *config_data,
>                                int config_len,
> @@ -1389,61 +1391,7 @@ static void parse_config_data(const char *config_source,
>                  if ((p2 = strchr(p, '=')) == NULL)
>                      break;
>                  *p2 = '\0';

You can get rid of p2, can't you?

[...]
> -            fprintf(stderr, "unrecognized argument `%s'\n", *argv);
> -            return 1;
> -        }
> +      if (set_device_data(&nic, &config, *argv)) { return 1;}

Coding style.

>      }
>  
>      if (dryrun_only) {
> -- 
> 1.9.1
> 
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel