[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] [PATCH 8/8] virsh: add -- support



On 10/12/2010 01:14 AM, Lai Jiangshan wrote:

"--" means no option at the following arguments.

Signed-off-by: Lai Jiangshan<laijs cn fujitsu com>
---
diff --git a/tools/virsh.c b/tools/virsh.c
index a5b438b..d01091f 100644
--- a/tools/virsh.c
+++ b/tools/virsh.c
@@ -10305,6 +10305,7 @@ vshCommandParse(vshControl *ctl, vshCommandParser *parser)
          vshCmdOpt *last = NULL;
          const vshCmdDef *cmd = NULL;
          int tk;
+        bool data_only = false;
          int data_ct = 0;

          first = NULL;
@@ -10327,6 +10328,8 @@ vshCommandParse(vshControl *ctl, vshCommandParser *parser)
                      goto syntaxError;   /* ... or ignore this command only? */
                  }
                  VIR_FREE(tkdata);
+            } else if (data_only) {
+                goto get_data;
              } else if (*tkdata == '-'&&  *(tkdata + 1) == '-'&&  *(tkdata + 2)
                         &&  c_isalnum(*(tkdata + 2))) {
                  char *optstr = strchr(tkdata + 2, '=');
@@ -10368,7 +10371,12 @@ vshCommandParse(vshControl *ctl, vshCommandParser *parser)
                          goto syntaxError;
                      }
                  }
+            } else if (*tkdata == '-'&&  *(tkdata + 1) == '-'
+&&  *(tkdata + 2) == '\0') {

Another case of line break convention. Also, I prefer tkdata[1] over *(tkdata + 1).

Hmm, that means there's now two levels of -- parsing - one to mark the end of top-level virsh commands, and one for each command. That is:

virsh -- help -- help

should be the same as

virsh help help

Which also means that virsh should probably be avoiding argv rearrangement in getopt_long(). That is, given my theoretical echo command,

virsh echo --help

should echo "--help", rather than running 'virsh --help'. Or, more to the point,

virsh dumpxml --update-cpu vm

correctly avoids promoting --update-cpu to a top-level argument of virsh.

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...

ACK with the nit fixed.  Here's what I'm squashing.

diff --git i/tools/virsh.c w/tools/virsh.c
index 79d7756..8c4a7bc 100644
--- i/tools/virsh.c
+++ w/tools/virsh.c
@@ -10347,8 +10347,8 @@ vshCommandParse(vshControl *ctl, vshCommandParser *parser)
                 VIR_FREE(tkdata);
             } else if (data_only) {
                 goto get_data;
- } else if (*tkdata == '-' && *(tkdata + 1) == '-' && *(tkdata + 2)
-                       && c_isalnum(*(tkdata + 2))) {
+            } else if (tkdata[0] == '-' && tkdata[1] == '-' &&
+                       c_isalnum(tkdata[2])) {
                 char *optstr = strchr(tkdata + 2, '=');
                 if (optstr) {
                     *optstr = '\0'; /* convert the '=' to '\0' */
@@ -10388,8 +10388,8 @@ vshCommandParse(vshControl *ctl, vshCommandParser *parser)
                         goto syntaxError;
                     }
                 }
-            } else if (*tkdata == '-' && *(tkdata + 1) == '-'
-                       && *(tkdata + 2) == '\0') {
+            } else if (tkdata[0] == '-' && tkdata[1] == '-' &&
+                       tkdata[2] == '\0') {
                 data_only = true;
                 continue;
             } else {


--
Eric Blake   eblake redhat com    +1-801-349-2682
Libvirt virtualization library http://libvirt.org


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]