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

[Eric Blake]

On 10/12/2010 01:14 AM, Lai Jiangshan wrote:
>
> "--" means no option at the following arguments.
>
> Signed-off-by: Lai Jiangshan<laijs at 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 at redhat.com    +1-801-349-2682
Libvirt virtualization library http://libvirt.org