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

[Xen-devel] [PATCH 1 of 2] xl: correct argument parsing for some sub-com

To: xen-devel@xxxxxxxxxxxxxxxxxxx
Subject: [Xen-devel] [PATCH 1 of 2] xl: correct argument parsing for some sub-commands
From: Ian Campbell <ian.campbell@xxxxxxxxxx>
Date: Mon, 23 Aug 2010 15:50:13 +0100
Cc: Ian Campbell <ian.campbell@xxxxxxxxxx>
Delivery-date: Mon, 23 Aug 2010 07:52:30 -0700
Envelope-to: www-data@xxxxxxxxxxxxxxxxxxx
In-reply-to: <patchbomb.1282575012@xxxxxxxxxxxxxxxxxxxxx>
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>
Sender: xen-devel-bounces@xxxxxxxxxxxxxxxxxxx
# HG changeset patch
# User Ian Campbell <ian.campbell@xxxxxxxxxx>
# Date 1282574054 -3600
# Node ID 2210bb76868ff58c1c97738f43c52b3e893dd178
# Parent  aa344916733f86fcde905953bad8b1cbc3020cd0
xl: correct argument parsing for some sub-commands.

XL sub-commands are expected to parse their arguments relative to the
global variable "optind" rather than treating argc+argv as zero
based. This is because the argc+argv passed to sub-commands include
the entire original command line, not just the sub command specific bits.

Not all commands do this and they are therefore broken if the user
uses "xl -v command", correct such problems

dump-core:
  - did not handle "-h" option.

{network,network2,block}-{attach,list,detach} :
  - handled arguments without reference to optind
  - checked number of arguments before processing getopt loop,
    breaking "-h" option handling

An example of the breakage:
    # xl -v block-list d32-2
    Vdev  BE  handle state evt-ch ring-ref BE-path
    block-list is an invalid domain identifier
    51712 0   1      4     13     8        /local/domain/0/backend/vbd/1/

Signed-off-by: Ian Campbell <ian.campbell@xxxxxxxxxx>

diff -r aa344916733f -r 2210bb76868f tools/libxl/xl_cmdimpl.c
--- a/tools/libxl/xl_cmdimpl.c  Mon Aug 23 09:46:03 2010 +0100
+++ b/tools/libxl/xl_cmdimpl.c  Mon Aug 23 15:34:14 2010 +0100
@@ -2870,6 +2870,17 @@ int main_migrate(int argc, char **argv)
 
 int main_dump_core(int argc, char **argv)
 {
+    int opt;
+    while ((opt = getopt(argc, argv, "h")) != -1) {
+        switch (opt) {
+        case 'h':
+            help("dump-core");
+            return 0;
+        default:
+            fprintf(stderr, "option `%c' not supported.\n", opt);
+            break;
+        }
+    }
     if ( argc-optind < 2 ) {
         help("dump-core");
         return 2;
@@ -4032,27 +4043,27 @@ int main_networkattach(int argc, char **
     int i;
     unsigned int val;
 
-    if ((argc < 3) || (argc > 11)) {
+    while ((opt = getopt(argc, argv, "h")) != -1) {
+        switch (opt) {
+        case 'h':
+            help("network-attach");
+            return 0;
+        default:
+            fprintf(stderr, "option `%c' not supported.\n", opt);
+            break;
+        }
+    }
+    if ((argc-optind < 2) || (argc-optind > 11)) {
         help("network-attach");
         return 0;
     }
-    while ((opt = getopt(argc, argv, "h")) != -1) {
-        switch (opt) {
-        case 'h':
-            help("network-attach");
-            return 0;
-        default:
-            fprintf(stderr, "option `%c' not supported.\n", opt);
-            break;
-        }
-    }
-
-    if (domain_qualifier_to_domid(argv[2], &domid, 0) < 0) {
-        fprintf(stderr, "%s is an invalid domain identifier\n", argv[2]);
+
+    if (domain_qualifier_to_domid(argv[optind], &domid, 0) < 0) {
+        fprintf(stderr, "%s is an invalid domain identifier\n", argv[optind]);
         return 1;
     }
     init_nic_info(&nic, -1);
-    for (argv += 3, argc -= 3; argc > 0; ++argv, --argc) {
+    for (argv += optind+1, argc -= optind+1; argc > 0; ++argv, --argc) {
         if (!strncmp("type=", *argv, 5)) {
             if (!strncmp("vif", (*argv) + 5, 4)) {
                 nic.nictype = NICTYPE_VIF;
@@ -4113,10 +4124,6 @@ int main_networklist(int argc, char **ar
     libxl_nicinfo *nics;
     unsigned int nb, i;
 
-    if (argc < 3) {
-        help("network-list");
-        return 1;
-    }
     while ((opt = getopt(argc, argv, "h")) != -1) {
         switch (opt) {
             case 'h':
@@ -4127,11 +4134,15 @@ int main_networklist(int argc, char **ar
                 break;
         }
     }
+    if (argc-optind < 1) {
+        help("network-list");
+        return 1;
+    }
 
     /*      Idx  BE   MAC   Hdl  Sta  evch txr/rxr  BE-path */
     printf("%-3s %-2s %-17s %-6s %-5s %-6s %5s/%-5s %-30s\n",
            "Idx", "BE", "Mac Addr.", "handle", "state", "evt-ch", "tx-", 
"rx-ring-ref", "BE-path");
-    for (argv += 2, argc -= 2; argc > 0; --argc, ++argv) {
+    for (argv += optind, argc -= optind; argc > 0; --argc, ++argv) {
         if (domain_qualifier_to_domid(*argv, &domid, 0) < 0) {
             fprintf(stderr, "%s is an invalid domain identifier\n", *argv);
             continue;
@@ -4162,34 +4173,34 @@ int main_networkdetach(int argc, char **
     int opt;
     libxl_device_nic nic;
 
-    if (argc != 4) {
+    while ((opt = getopt(argc, argv, "h")) != -1) {
+        switch (opt) {
+        case 'h':
+            help("network-detach");
+            return 0;
+        default:
+            fprintf(stderr, "option `%c' not supported.\n", opt);
+            break;
+        }
+    }
+    if (argc-optind != 2) {
         help("network-detach");
         return 0;
     }
-    while ((opt = getopt(argc, argv, "h")) != -1) {
-        switch (opt) {
-        case 'h':
-            help("network-detach");
-            return 0;
-        default:
-            fprintf(stderr, "option `%c' not supported.\n", opt);
-            break;
-        }
-    }
-
-    if (domain_qualifier_to_domid(argv[2], &domid, 0) < 0) {
-        fprintf(stderr, "%s is an invalid domain identifier\n", argv[2]);
-        return 1;
-    }
-
-    if (!strchr(argv[3], ':')) {
-        if (libxl_devid_to_device_nic(&ctx, domid, argv[3], &nic)) {
-            fprintf(stderr, "Unknown device %s.\n", argv[3]);
-            return 1;
-        }
-    } else {
-        if (libxl_mac_to_device_nic(&ctx, domid, argv[3], &nic)) {
-            fprintf(stderr, "Unknown device %s.\n", argv[3]);
+
+    if (domain_qualifier_to_domid(argv[optind], &domid, 0) < 0) {
+        fprintf(stderr, "%s is an invalid domain identifier\n", argv[optind]);
+        return 1;
+    }
+
+    if (!strchr(argv[optind+1], ':')) {
+        if (libxl_devid_to_device_nic(&ctx, domid, argv[optind+1], &nic)) {
+            fprintf(stderr, "Unknown device %s.\n", argv[optind+1]);
+            return 1;
+        }
+    } else {
+        if (libxl_mac_to_device_nic(&ctx, domid, argv[optind+1], &nic)) {
+            fprintf(stderr, "Unknown device %s.\n", argv[optind+1]);
             return 1;
         }
     }
@@ -4208,22 +4219,22 @@ int main_blockattach(int argc, char **ar
     uint32_t fe_domid, be_domid = 0;
     libxl_device_disk disk = { 0 };
 
-    if ((argc < 5) || (argc > 7)) {
+    while ((opt = getopt(argc, argv, "h")) != -1) {
+        switch (opt) {
+        case 'h':
+            help("block-attach");
+            return 0;
+        default:
+            fprintf(stderr, "option `%c' not supported.\n", opt);
+            break;
+        }
+    }
+    if ((argc-optind < 3) || (argc-optind > 5)) {
         help("block-attach");
         return 0;
     }
-    while ((opt = getopt(argc, argv, "h")) != -1) {
-        switch (opt) {
-        case 'h':
-            help("block-attach");
-            return 0;
-        default:
-            fprintf(stderr, "option `%c' not supported.\n", opt);
-            break;
-        }
-    }
-
-    tok = strtok(argv[3], ":");
+
+    tok = strtok(argv[optind+1], ":");
     if (!strcmp(tok, "phy")) {
         disk.phystype = PHYSTYPE_PHY;
     } else if (!strcmp(tok, "file")) {
@@ -4251,22 +4262,23 @@ int main_blockattach(int argc, char **ar
         fprintf(stderr, "Error: missing path to disk image.\n");
         return 1;
     }
-    disk.virtpath = argv[4];
+    disk.virtpath = argv[optind+2];
     disk.unpluggable = 1;
-    disk.readwrite = ((argc <= 4) || (argv[5][0] == 'w'));
-
-    if (domain_qualifier_to_domid(argv[2], &fe_domid, 0) < 0) {
-        fprintf(stderr, "%s is an invalid domain identifier\n", argv[2]);
-        return 1;
-    }
-    if (argc == 7) {
-        if (domain_qualifier_to_domid(argv[6], &be_domid, 0) < 0) {
-            fprintf(stderr, "%s is an invalid domain identifier\n", argv[6]);
+    disk.readwrite = ((argc-optind <= 2) || (argv[optind+3][0] == 'w'));
+
+    if (domain_qualifier_to_domid(argv[optind], &fe_domid, 0) < 0) {
+        fprintf(stderr, "%s is an invalid domain identifier\n", argv[optind]);
+        return 1;
+    }
+    if (argc-optind == 5) {
+        if (domain_qualifier_to_domid(argv[optind+4], &be_domid, 0) < 0) {
+            fprintf(stderr, "%s is an invalid domain identifier\n", 
argv[optind+4]);
             return 1;
         }
     }
     disk.domid = fe_domid;
     disk.backend_domid = be_domid;
+
     if (libxl_device_disk_add(&ctx, fe_domid, &disk)) {
         fprintf(stderr, "libxl_device_disk_add failed.\n");
     }
@@ -4280,24 +4292,24 @@ int main_blocklist(int argc, char **argv
     libxl_device_disk *disks;
     libxl_diskinfo diskinfo;
 
-    if (argc < 3) {
+    while ((opt = getopt(argc, argv, "h")) != -1) {
+        switch (opt) {
+        case 'h':
+            help("block-list");
+            return 0;
+        default:
+            fprintf(stderr, "option `%c' not supported.\n", opt);
+            break;
+        }
+    }
+    if (argc-optind < 1) {
         help("block-list");
         return 0;
-    }
-    while ((opt = getopt(argc, argv, "h")) != -1) {
-        switch (opt) {
-        case 'h':
-            help("block-list");
-            return 0;
-        default:
-            fprintf(stderr, "option `%c' not supported.\n", opt);
-            break;
-        }
     }
 
     printf("%-5s %-3s %-6s %-5s %-6s %-8s %-30s\n",
            "Vdev", "BE", "handle", "state", "evt-ch", "ring-ref", "BE-path");
-    for (argv += 2, argc -= 2; argc > 0; --argc, ++argv) {
+    for (argv += optind, argc -= optind; argc > 0; --argc, ++argv) {
         if (domain_qualifier_to_domid(*argv, &domid, 0) < 0) {
             fprintf(stderr, "%s is an invalid domain identifier\n", *argv);
             continue;
@@ -4326,27 +4338,27 @@ int main_blockdetach(int argc, char **ar
     int opt;
     libxl_device_disk disk;
 
-    if (argc != 4) {
+    while ((opt = getopt(argc, argv, "h")) != -1) {
+        switch (opt) {
+        case 'h':
+            help("block-detach");
+            return 0;
+        default:
+            fprintf(stderr, "option `%c' not supported.\n", opt);
+            break;
+        }
+    }
+    if (argc-optind != 2) {
         help("block-detach");
         return 0;
     }
-    while ((opt = getopt(argc, argv, "h")) != -1) {
-        switch (opt) {
-        case 'h':
-            help("block-detach");
-            return 0;
-        default:
-            fprintf(stderr, "option `%c' not supported.\n", opt);
-            break;
-        }
-    }
-
-    if (domain_qualifier_to_domid(argv[2], &domid, 0) < 0) {
-        fprintf(stderr, "%s is an invalid domain identifier\n", argv[2]);
-        return 1;
-    }
-    if (libxl_devid_to_device_disk(&ctx, domid, argv[3], &disk)) {
-        fprintf(stderr, "Error: Device %s not connected.\n", argv[3]);
+
+    if (domain_qualifier_to_domid(argv[optind], &domid, 0) < 0) {
+        fprintf(stderr, "%s is an invalid domain identifier\n", argv[optind]);
+        return 1;
+    }
+    if (libxl_devid_to_device_disk(&ctx, domid, argv[optind+1], &disk)) {
+        fprintf(stderr, "Error: Device %s not connected.\n", argv[optind+1]);
         return 1;
     }
     if (libxl_device_disk_del(&ctx, &disk, 1)) {
@@ -4364,27 +4376,27 @@ int main_network2attach(int argc, char *
     unsigned int val, i;
     libxl_device_net2 net2;
 
-    if ((argc < 3) || (argc > 12)) {
+    while ((opt = getopt(argc, argv, "h")) != -1) {
+        switch (opt) {
+        case 'h':
+            help("network2-attach");
+            return 0;
+        default:
+            fprintf(stderr, "option `%c' not supported.\n", opt);
+            break;
+        }
+    }
+    if ((argc-optind < 1) || (argc-optind > 10)) {
         help("network2-attach");
         return 0;
     }
-    while ((opt = getopt(argc, argv, "h")) != -1) {
-        switch (opt) {
-        case 'h':
-            help("network2-attach");
-            return 0;
-        default:
-            fprintf(stderr, "option `%c' not supported.\n", opt);
-            break;
-        }
-    }
-
-    if (domain_qualifier_to_domid(argv[2], &domid, 0) < 0) {
-        fprintf(stderr, "%s is an invalid domain identifier\n", argv[1]);
+
+    if (domain_qualifier_to_domid(argv[optind], &domid, 0) < 0) {
+        fprintf(stderr, "%s is an invalid domain identifier\n", argv[optind]);
         return 1;
     }
     init_net2_info(&net2, -1);
-    for (argv += 3, argc -= 3; argc > 0; --argc, ++argv) {
+    for (argv += optind+1, argc -= optind+1; argc > 0; --argc, ++argv) {
         if (!strncmp("front_mac=", *argv, 10)) {
             tok = strtok((*argv) + 10, ":");
             for (i = 0; tok && i < 6; tok = strtok(NULL, ":"), ++i) {
@@ -4457,25 +4469,25 @@ int main_network2list(int argc, char **a
     unsigned int nb;
     libxl_net2info *net2s;
 
-    if (argc < 3) {
+    while ((opt = getopt(argc, argv, "h")) != -1) {
+        switch (opt) {
+        case 'h':
+            help("network2-list");
+            return 0;
+        default:
+            fprintf(stderr, "option `%c' not supported.\n", opt);
+            break;
+        }
+    }
+    if (argc - optind < 1) {
         help("network2-list");
         return 0;
-    }
-    while ((opt = getopt(argc, argv, "h")) != -1) {
-        switch (opt) {
-        case 'h':
-            help("network2-list");
-            return 0;
-        default:
-            fprintf(stderr, "option `%c' not supported.\n", opt);
-            break;
-        }
     }
 
     printf("%-3s %-2s %-5s %-17s %-17s %-7s %-6s %-30s\n",
            "Idx", "BE", "state", "Mac Addr.", "Remote Mac Addr.",
            "trusted", "filter", "backend");
-    for (argv += 2, argc -=2; argc > 0; --argc, ++argv) {
+    for (argv += optind, argc -=optind; argc > 0; --argc, ++argv) {
         if (domain_qualifier_to_domid(*argv, &domid, 0) < 0) {
             fprintf(stderr, "%s is an invalid domain identifier\n", *argv);
             continue;
@@ -4501,27 +4513,27 @@ int main_network2detach(int argc, char *
     int opt;
     libxl_device_net2 net2;
 
-    if (argc != 4) {
+    while ((opt = getopt(argc, argv, "h")) != -1) {
+        switch (opt) {
+        case 'h':
+            help("network2-detach");
+            return 0;
+        default:
+            fprintf(stderr, "option `%c' not supported.\n", opt);
+            break;
+        }
+    }
+    if (argc-optind != 2) {
         help("network2-detach");
         return 0;
     }
-    while ((opt = getopt(argc, argv, "h")) != -1) {
-        switch (opt) {
-        case 'h':
-            help("network2-detach");
-            return 0;
-        default:
-            fprintf(stderr, "option `%c' not supported.\n", opt);
-            break;
-        }
-    }
-
-    if (domain_qualifier_to_domid(argv[2], &domid, 0) < 0) {
-        fprintf(stderr, "%s is an invalid domain identifier\n", argv[2]);
-        return 1;
-    }
-    if (libxl_devid_to_device_net2(&ctx, domid, argv[3], &net2)) {
-       fprintf(stderr, "Error: Device %s not connected.\n", argv[3]);
+
+    if (domain_qualifier_to_domid(argv[optind], &domid, 0) < 0) {
+        fprintf(stderr, "%s is an invalid domain identifier\n", argv[optind]);
+        return 1;
+    }
+    if (libxl_devid_to_device_net2(&ctx, domid, argv[optind+1], &net2)) {
+        fprintf(stderr, "Error: Device %s not connected.\n", argv[optind+1]);
         return 1;
     }
     if (libxl_device_net2_del(&ctx, &net2, 1)) {
diff -r aa344916733f -r 2210bb76868f tools/libxl/xl_cmdtable.c
--- a/tools/libxl/xl_cmdtable.c Mon Aug 23 09:46:03 2010 +0100
+++ b/tools/libxl/xl_cmdtable.c Mon Aug 23 15:34:14 2010 +0100
@@ -241,7 +241,7 @@ struct cmd_spec cmd_table[] = {
       "Create a new virtual network device",
       "<Domain> [type=<type>] [mac=<mac>] [bridge=<bridge>] "
       "[ip=<ip>] [script=<script>] [backend=<BackDomain>] [vifname=<name>] "
-      "[rate=<rate>] [model=<model>][accel=<accel>]",
+      "[rate=<rate>] [model=<model>] [accel=<accel>]",
     },
     { "network-list",
       &main_networklist,

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

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