WARNING - OLD ARCHIVES

This is an archived copy of the Xen.org mailing list, which we have preserved to ensure that existing links to archives are not broken. The live archive, which contains the latest emails, can be found at http://lists.xen.org/
   
 
 
Xen 
 
Home Products Support Community News
 
   
 

xen-devel

Re: [Xen-devel] [PATCH 4 of 6 RESENT] xl: Use macros for param parsing i

To: Marek Marczykowski <marmarek@xxxxxxxxxxxx>
Subject: Re: [Xen-devel] [PATCH 4 of 6 RESENT] xl: Use macros for param parsing in network-attach, to prevent use explicit sizeof("param")
From: Stefano Stabellini <stefano.stabellini@xxxxxxxxxxxxx>
Date: Tue, 7 Jun 2011 13:02:08 +0100
Cc: "xen-devel@xxxxxxxxxxxxxxxxxxx" <xen-devel@xxxxxxxxxxxxxxxxxxx>
Delivery-date: Tue, 07 Jun 2011 05:03:17 -0700
Envelope-to: www-data@xxxxxxxxxxxxxxxxxxx
In-reply-to: <0863dfd23b3a10bee6d3.1307292634@devel14>
List-help: <mailto:xen-devel-request@lists.xensource.com?subject=help>
List-id: Xen developer discussion <xen-devel.lists.xensource.com>
List-post: <mailto:xen-devel@lists.xensource.com>
List-subscribe: <http://lists.xensource.com/mailman/listinfo/xen-devel>, <mailto:xen-devel-request@lists.xensource.com?subject=subscribe>
List-unsubscribe: <http://lists.xensource.com/mailman/listinfo/xen-devel>, <mailto:xen-devel-request@lists.xensource.com?subject=unsubscribe>
References: <patchbomb.1307292630@devel14> <0863dfd23b3a10bee6d3.1307292634@devel14>
Sender: xen-devel-bounces@xxxxxxxxxxxxxxxxxxx
User-agent: Alpine 2.00 (DEB 1167 2008-08-23)
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

<Prev in Thread] Current Thread [Next in Thread>