Re: [libvirt] [PATCH] virsh: simplify top-level option parsing

2010-10-13 Thread Daniel Veillard
On Tue, Oct 12, 2010 at 03:51:48PM -0600, Eric Blake wrote:
 This makes 'virsh --conn test:///default help help' work right;
 previously, the abbreviation confused our hand-rolled option parsing.
 
 * tools/virsh.c (vshParseArgv): Use getopt_long feature, rather
 than (incorrectly) reparsing options ourselves.
 ---
 
  Oh my - I just looked in the code, and virsh is re-doing option
  parsing by itself, instead of just telling getopt_long() to stop on
  the first non-option; but getting it wrong by not checking for
  abbreviations. Another patch or two coming up...
 
 I love patches that nuke more code than they add, all while fixing
 bugs at the same time!
 
  tools/virsh.c |   68 +---
  1 files changed, 16 insertions(+), 52 deletions(-)

  ACK, way cleaner !

Daniel

-- 
Daniel Veillard  | libxml Gnome XML XSLT toolkit  http://xmlsoft.org/
dan...@veillard.com  | Rpmfind RPM search engine http://rpmfind.net/
http://veillard.com/ | virtualization library  http://libvirt.org/

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


[libvirt] [PATCH] virsh: simplify top-level option parsing

2010-10-12 Thread Eric Blake
This makes 'virsh --conn test:///default help help' work right;
previously, the abbreviation confused our hand-rolled option parsing.

* tools/virsh.c (vshParseArgv): Use getopt_long feature, rather
than (incorrectly) reparsing options ourselves.
---

 Oh my - I just looked in the code, and virsh is re-doing option
 parsing by itself, instead of just telling getopt_long() to stop on
 the first non-option; but getting it wrong by not checking for
 abbreviations. Another patch or two coming up...

I love patches that nuke more code than they add, all while fixing
bugs at the same time!

 tools/virsh.c |   68 +---
 1 files changed, 16 insertions(+), 52 deletions(-)

diff --git a/tools/virsh.c b/tools/virsh.c
index 8c4a7bc..19a6087 100644
--- a/tools/virsh.c
+++ b/tools/virsh.c
@@ -10991,9 +10991,8 @@ vshUsage(void)
 static int
 vshParseArgv(vshControl *ctl, int argc, char **argv)
 {
-char *last = NULL;
-int i, end = 0, help = 0;
-int arg, idx = 0;
+bool help = false;
+int arg;
 struct option opt[] = {
 {debug, 1, 0, 'd'},
 {help, 0, 0, 'h'},
@@ -11006,46 +11005,10 @@ vshParseArgv(vshControl *ctl, int argc, char **argv)
 {0, 0, 0, 0}
 };

-
-if (argc  2)
-return TRUE;
-
-/* look for begin of the command, for example:
- *   ./virsh --debug 5 -q command --cmdoption
- *  --- ^ ---
- *getopt() stuff | command suff
- */
-for (i = 1; i  argc; i++) {
-if (*argv[i] != '-') {
-int valid = FALSE;
-
-/* non --option argv, is it command? */
-if (last) {
-struct option *o;
-int sz = strlen(last);
-
-for (o = opt; o-name; o++) {
-if (o-has_arg == 1){
-if (sz == 2  *(last + 1) == o-val)
-/* valid virsh short option */
-valid = TRUE;
-else if (sz  2  STREQ(o-name, last + 2))
-/* valid virsh long option */
-valid = TRUE;
-}
-}
-}
-if (!valid) {
-end = i;
-break;
-}
-}
-last = argv[i];
-}
-end = end ? end : argc;
-
-/* standard (non-command) options */
-while ((arg = getopt_long(end, argv, d:hqtc:vrl:, opt, idx)) != -1) {
+/* Standard (non-command) options. The leading + ensures that no
+ * argument reordering takes place, so that command options are
+ * not confused with top-level virsh options. */
+while ((arg = getopt_long(argc, argv, +d:hqtc:vrl:, opt, NULL)) != -1) {
 switch (arg) {
 case 'd':
 if (virStrToLong_i(optarg, NULL, 10, ctl-debug)  0) {
@@ -11054,7 +11017,7 @@ vshParseArgv(vshControl *ctl, int argc, char **argv)
 }
 break;
 case 'h':
-help = 1;
+help = true;
 break;
 case 'q':
 ctl-quiet = TRUE;
@@ -11066,7 +11029,8 @@ vshParseArgv(vshControl *ctl, int argc, char **argv)
 ctl-name = vshStrdup(ctl, optarg);
 break;
 case 'v':
-fprintf(stdout, %s\n, VERSION);
+/* FIXME - list a copyright blurb, as in GNU programs?  */
+puts(VERSION);
 exit(EXIT_SUCCESS);
 case 'r':
 ctl-readonly = TRUE;
@@ -11081,8 +11045,8 @@ vshParseArgv(vshControl *ctl, int argc, char **argv)
 }

 if (help) {
-if (end  argc) {
-vshError(ctl, _(extra argument '%s'. See --help.), argv[end]);
+if (optind  argc) {
+vshError(ctl, _(extra argument '%s'. See --help.), argv[optind]);
 exit(EXIT_FAILURE);
 }

@@ -11091,14 +11055,14 @@ vshParseArgv(vshControl *ctl, int argc, char **argv)
 exit(EXIT_SUCCESS);
 }

-if (argc  end) {
+if (argc  optind) {
 /* parse command */
 ctl-imode = FALSE;
-if (argc - end == 1) {
-vshDebug(ctl, 2, commands: \%s\\n, argv[end]);
-return vshCommandStringParse(ctl, argv[end]);
+if (argc - optind == 1) {
+vshDebug(ctl, 2, commands: \%s\\n, argv[optind]);
+return vshCommandStringParse(ctl, argv[optind]);
 } else {
-return vshCommandArgvParse(ctl, argc - end, argv + end);
+return vshCommandArgvParse(ctl, argc - optind, argv + optind);
 }
 }
 return TRUE;
-- 
1.7.2.3

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list