Re: [Xen-devel] [PATCH 4 of 6 RESENT] xl: Use macros for param parsing in network-attach, to prevent use explicit sizeof("param")

[Stefano Stabellini <stefano.stabellini@xxxxxxxxxxxxx>]

On Sun, 5 Jun 2011, Marek Marczykowski wrote:
> # HG changeset patch
> # User Marek Marczykowski <marmarek@xxxxxxxxxxxx>
> # Date 1307145042 -7200
> # Node ID 0863dfd23b3a10bee6d3eda30415dff0eef2bee1
> # Parent  9fe949c7ab9601bb5500a53c538f7a23b61e1bcb
> xl: Use macros for param parsing in network-attach, to prevent use explicit 
> sizeof("param")
> 
> 'script=' length was wrong... Replaced calls to strncmp("param", *argv,
> explicit sizeof("param")) with macro and helper function to extract parameter
> value. Also introduce replace_string function to simplify code.
> 
> Signed-off-by: Marek Marczykowski <marmarek@xxxxxxxxxxxx>

It seems correct to me and makes the code easier to read, so

Acked-by: Stefano Stabellini <stefano.stabellini@xxxxxxxxxxxxx>


> diff -r 9fe949c7ab96 -r 0863dfd23b3a tools/libxl/xl_cmdimpl.c
> --- a/tools/libxl/xl_cmdimpl.c        Wed Jun 01 23:15:54 2011 +0200
> +++ b/tools/libxl/xl_cmdimpl.c        Sat Jun 04 01:50:42 2011 +0200
> @@ -1229,6 +1229,24 @@ static int handle_domain_death(libxl_ctx
>      return restart;
>  }
>  
> +/* for now used only by main_networkattach, but can be reused elsewhere */
> +static int match_option_size(const char *prefix, size_t len,
> +        char *arg, char **argopt)
> +{
> +    int rc = strncmp(prefix, arg, len);
> +    if (!rc) *argopt = arg+len;
> +    return !rc;
> +}
> +#define match_option(_prefix, _arg, _oparg) \
> +    match_option_size((_prefix "="), sizeof((_prefix)) + 1, (_arg), 
> &(_oparg))
> +
> +static void replace_string(char **str, const char *val)
> +{
> +    free(*str);
> +    *str = strdup(val);
> +}
> +
> +
>  static int preserve_domain(libxl_ctx *ctx, uint32_t domid, libxl_event 
> *event,
>                             libxl_domain_config *d_config, libxl_dominfo 
> *info)
>  {
> @@ -3925,7 +3943,7 @@ int main_networkattach(int argc, char **
>  {
>      int opt;
>      libxl_device_nic nic;
> -    char *endptr;
> +    char *endptr, *oparg;
>      const char *tok;
>      int i;
>      unsigned int val;
> @@ -3944,17 +3962,17 @@ int main_networkattach(int argc, char **
>      }
>      libxl_device_nic_init(&nic, -1);
>      for (argv += optind+1, argc -= optind+1; argc > 0; ++argv, --argc) {
> -        if (!strncmp("type=", *argv, 5)) {
> -            if (!strncmp("vif", (*argv) + 5, 4)) {
> +        if (match_option("type", *argv, oparg)) {
> +            if (!strcmp("vif", oparg)) {
>                  nic.nictype = LIBXL_NIC_TYPE_VIF;
> -            } else if (!strncmp("ioemu", (*argv) + 5, 5)) {
> +            } else if (!strcmp("ioemu", oparg)) {
>                  nic.nictype = LIBXL_NIC_TYPE_IOEMU;
>              } else {
>                  fprintf(stderr, "Invalid parameter `type'.\n");
>                  return 1;
>              }
> -        } else if (!strncmp("mac=", *argv, 4)) {
> -            tok = strtok((*argv) + 4, ":");
> +        } else if (match_option("mac", *argv, oparg)) {
> +            tok = strtok(oparg, ":");
>              for (i = 0; tok && i < 6; tok = strtok(NULL, ":"), ++i) {
>                  val = strtoul(tok, &endptr, 16);
>                  if ((tok == endptr) || (val > 255)) {
> @@ -3963,29 +3981,24 @@ int main_networkattach(int argc, char **
>                  }
>                  nic.mac[i] = val;
>              }
> -        } else if (!strncmp("bridge=", *argv, 7)) {
> -            free(nic.bridge);
> -            nic.bridge = strdup((*argv) + 7);
> -        } else if (!strncmp("ip=", *argv, 3)) {
> -            free(nic.ip);
> -            nic.ip = strdup((*argv) + 3);
> -        } else if (!strncmp("script=", *argv, 6)) {
> -            free(nic.script);
> -            nic.script = strdup((*argv) + 6);
> -        } else if (!strncmp("backend=", *argv, 8)) {
> -            if(libxl_name_to_domid(ctx, ((*argv) + 8), &val)) {
> +        } else if (match_option("bridge", *argv, oparg)) {
> +            replace_string(&nic.bridge, oparg);
> +        } else if (match_option("ip", *argv, oparg)) {
> +            replace_string(&nic.ip, oparg);
> +        } else if (match_option("script", *argv, oparg)) {
> +            replace_string(&nic.script, oparg);
> +        } else if (match_option("backend", *argv, oparg)) {
> +            if(libxl_name_to_domid(ctx, oparg, &val)) {
>                  fprintf(stderr, "Specified backend domain does not exist, 
> defaulting to Dom0\n");
>                  val = 0;
>              }
>              nic.backend_domid = val;
> -        } else if (!strncmp("vifname=", *argv, 8)) {
> -            free(nic.ifname);
> -            nic.ifname = strdup((*argv) + 8);
> -        } else if (!strncmp("model=", *argv, 6)) {
> -            free(nic.model);
> -            nic.model = strdup((*argv) + 6);
> -        } else if (!strncmp("rate=", *argv, 5)) {
> -        } else if (!strncmp("accel=", *argv, 6)) {
> +        } else if (match_option("vifname", *argv, oparg)) {
> +            replace_string(&nic.ifname, oparg);
> +        } else if (match_option("model", *argv, oparg)) {
> +            replace_string(&nic.model, oparg);
> +        } else if (match_option("rate", *argv, oparg)) {
> +        } else if (match_option("accel", *argv, oparg)) {
>          } else {
>              fprintf(stderr, "unrecognized argument `%s'\n", *argv);
>              return 1;
> 
> 
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@xxxxxxxxxxxxxxxxxxx
> http://lists.xensource.com/xen-devel
> 

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel