[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] [Xen-devel] [PATCH 1 of 2] xl: correct argument parsing for some sub-commands
# 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
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |