From: Ian Campbell <ian.campbell@citrix.com>
To: xen-devel@lists.xensource.com
Cc: Ian Campbell <ian.campbell@citrix.com>
Subject: [PATCH 1 of 2] xl: correct argument parsing for some sub-commands
Date: Mon, 23 Aug 2010 15:50:13 +0100 [thread overview]
Message-ID: <2210bb76868ff58c1c97.1282575013@localhost.localdomain> (raw)
In-Reply-To: <patchbomb.1282575012@localhost.localdomain>
# HG changeset patch
# User Ian Campbell <ian.campbell@citrix.com>
# 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@citrix.com>
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,
next prev parent reply other threads:[~2010-08-23 14:50 UTC|newest]
Thread overview: 3+ messages / expand[flat|nested] mbox.gz Atom feed top
2010-08-23 14:50 [PATCH 0 of 2] xl: fixes and improvements to xl command line parsing Ian Campbell
2010-08-23 14:50 ` Ian Campbell [this message]
2010-08-23 14:50 ` [PATCH 2 of 2] xl: treat sub-command main function like a regular C main() function Ian Campbell
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=2210bb76868ff58c1c97.1282575013@localhost.localdomain \
--to=ian.campbell@citrix.com \
--cc=xen-devel@lists.xensource.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.