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

On 10/12/2010 01:14 AM, Lai Jiangshan wrote: 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(a)redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org