[libvirt] [PATCH 1/3] virsh.c: Fix compiler warning

For some reason I only get this after applying subsequent upcoming patches that touch virsh, but don't seem to actually cause the warning. virsh.c: In function ‘vshCommandParse’: virsh.c:2014:46: error: ‘opt_index’ may be used uninitialized in this function [-Werror=maybe-uninitialized] cc1: all warnings being treated as errors --- tools/virsh.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tools/virsh.c b/tools/virsh.c index b95a008..64e2e18 100644 --- a/tools/virsh.c +++ b/tools/virsh.c @@ -1975,7 +1975,7 @@ vshCommandParse(vshControl *ctl, vshCommandParser *parser) } else if (tkdata[0] == '-' && tkdata[1] == '-' && c_isalnum(tkdata[2])) { char *optstr = strchr(tkdata + 2, '='); - int opt_index; + int opt_index = 0; if (optstr) { *optstr = '\0'; /* convert the '=' to '\0' */ -- 1.7.11.2

Often times I find myself halfway through typing a long command when I want to see 'help' output. I instinctively append '--help' to the command I'm typing, only to get an error: $ virsh vol-create-as foo --help error: command 'vol-create-as' doesn't support option --help This patch makes --help work in a pretty hacky way. One missing piece here is that --help isn't listed as an option in the actual 'help <cmd>' output, but maybe this can be a starting point for someone. --- tools/virsh.c | 25 +++++++++++++++++++++++++ 1 file changed, 25 insertions(+) diff --git a/tools/virsh.c b/tools/virsh.c index 64e2e18..324f789 100644 --- a/tools/virsh.c +++ b/tools/virsh.c @@ -1189,12 +1189,18 @@ vshCmddefOptParse(const vshCmdDef *cmd, uint32_t *opts_need_arg, return 0; } +vshCmdOptDef helpopt = {"help", VSH_OT_BOOL, 0, + N_("print help for this function")}; static const vshCmdOptDef * vshCmddefGetOption(vshControl *ctl, const vshCmdDef *cmd, const char *name, uint32_t *opts_seen, int *opt_index) { int i; + if (STREQ(name, helpopt.name)) { + return &helpopt; + } + for (i = 0; cmd->opts && cmd->opts[i].name; i++) { const vshCmdOptDef *opt = &cmd->opts[i]; @@ -2053,6 +2059,25 @@ get_data: /* command parsed -- allocate new struct for the command */ if (cmd) { vshCmd *c = vshMalloc(ctl, sizeof(vshCmd)); + vshCmdOpt *tmpopt = first; + + /* if we encountered --help, replace parsed command with + * 'help <cmdname>' */ + for (tmpopt = first; tmpopt; tmpopt = tmpopt->next) { + if (STRNEQ(tmpopt->def->name, "help")) + continue; + + vshCommandOptFree(first); + first = vshMalloc(ctl, sizeof(vshCmdOpt)); + first->def = &(opts_help[0]); + first->data = vshStrdup(ctl, cmd->name); + first->next = NULL; + + cmd = vshCmddefSearch("help"); + opts_required = 0; + opts_seen = 0; + break; + } c->opts = first; c->def = cmd; -- 1.7.11.2

On 08/13/2012 08:28 AM, Cole Robinson wrote:
Often times I find myself halfway through typing a long command when I want to see 'help' output. I instinctively append '--help' to the command I'm typing, only to get an error:
$ virsh vol-create-as foo --help error: command 'vol-create-as' doesn't support option --help
This patch makes --help work in a pretty hacky way. One missing piece here is that --help isn't listed as an option in the actual 'help <cmd>' output, but maybe this can be a starting point for someone.
Love this hack - I too have missed having it.
--- tools/virsh.c | 25 +++++++++++++++++++++++++ 1 file changed, 25 insertions(+)
+vshCmdOptDef helpopt = {"help", VSH_OT_BOOL, 0, + N_("print help for this function")};
Mark this static.
static const vshCmdOptDef * vshCmddefGetOption(vshControl *ctl, const vshCmdDef *cmd, const char *name, uint32_t *opts_seen, int *opt_index) { int i;
+ if (STREQ(name, helpopt.name)) { + return &helpopt; + } + for (i = 0; cmd->opts && cmd->opts[i].name; i++) { const vshCmdOptDef *opt = &cmd->opts[i];
@@ -2053,6 +2059,25 @@ get_data: /* command parsed -- allocate new struct for the command */ if (cmd) { vshCmd *c = vshMalloc(ctl, sizeof(vshCmd)); + vshCmdOpt *tmpopt = first; + + /* if we encountered --help, replace parsed command with + * 'help <cmdname>' */ + for (tmpopt = first; tmpopt; tmpopt = tmpopt->next) { + if (STRNEQ(tmpopt->def->name, "help")) + continue;
I tested corner cases like: virsh echo --help # full help output virsh echo -- --help # literal '--help' virsh echo --string --help # literal '--help' and it appears to match my expectations. ACK. -- Eric Blake eblake@redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org

On Mon, Aug 13, 2012 at 9:28 AM, Cole Robinson <crobinso@redhat.com> wrote:
Often times I find myself halfway through typing a long command when I want to see 'help' output. I instinctively append '--help' to the command I'm typing, only to get an error:
$ virsh vol-create-as foo --help error: command 'vol-create-as' doesn't support option --help
This patch makes --help work in a pretty hacky way. One missing piece here is that --help isn't listed as an option in the actual 'help <cmd>' output, but maybe this can be a starting point for someone. --- tools/virsh.c | 25 +++++++++++++++++++++++++ 1 file changed, 25 insertions(+)
diff --git a/tools/virsh.c b/tools/virsh.c index 64e2e18..324f789 100644 --- a/tools/virsh.c +++ b/tools/virsh.c @@ -1189,12 +1189,18 @@ vshCmddefOptParse(const vshCmdDef *cmd, uint32_t *opts_need_arg, return 0; }
+vshCmdOptDef helpopt = {"help", VSH_OT_BOOL, 0, + N_("print help for this function")}; static const vshCmdOptDef * vshCmddefGetOption(vshControl *ctl, const vshCmdDef *cmd, const char *name, uint32_t *opts_seen, int *opt_index) { int i;
+ if (STREQ(name, helpopt.name)) { + return &helpopt; + } + for (i = 0; cmd->opts && cmd->opts[i].name; i++) { const vshCmdOptDef *opt = &cmd->opts[i];
@@ -2053,6 +2059,25 @@ get_data: /* command parsed -- allocate new struct for the command */ if (cmd) { vshCmd *c = vshMalloc(ctl, sizeof(vshCmd)); + vshCmdOpt *tmpopt = first; + + /* if we encountered --help, replace parsed command with + * 'help <cmdname>' */ + for (tmpopt = first; tmpopt; tmpopt = tmpopt->next) { + if (STRNEQ(tmpopt->def->name, "help")) + continue; + + vshCommandOptFree(first); + first = vshMalloc(ctl, sizeof(vshCmdOpt)); + first->def = &(opts_help[0]); + first->data = vshStrdup(ctl, cmd->name); + first->next = NULL; + + cmd = vshCmddefSearch("help"); + opts_required = 0; + opts_seen = 0; + break; + }
c->opts = first; c->def = cmd; -- 1.7.11.2
I love this feature so much. Changes look good. ACK. -- Doug Goldstein

Similar to the previous patch, prepending 'help' to a partial command string doesn't cut us any slack. $ virsh help pool-define-as --name foo --type dir error: command 'help' doesn't support option --name This patch adds a few hacks to make 'help' ignore everything after the first data bit, so the above command shows help output for pool-define-as. --- tools/virsh.c | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/tools/virsh.c b/tools/virsh.c index 324f789..ab8d6dc 100644 --- a/tools/virsh.c +++ b/tools/virsh.c @@ -1219,8 +1219,10 @@ vshCmddefGetOption(vshControl *ctl, const vshCmdDef *cmd, const char *name, } } - vshError(ctl, _("command '%s' doesn't support option --%s"), - cmd->name, name); + if (STRNEQ(cmd->name, "help")) { + vshError(ctl, _("command '%s' doesn't support option --%s"), + cmd->name, name); + } return NULL; } @@ -1987,9 +1989,12 @@ vshCommandParse(vshControl *ctl, vshCommandParser *parser) *optstr = '\0'; /* convert the '=' to '\0' */ optstr = vshStrdup(ctl, optstr + 1); } + /* Special case 'help' to ignore all spurious options */ if (!(opt = vshCmddefGetOption(ctl, cmd, tkdata + 2, &opts_seen, &opt_index))) { VIR_FREE(optstr); + if (STREQ(cmd->name, "help")) + continue; goto syntaxError; } VIR_FREE(tkdata); @@ -2027,8 +2032,10 @@ vshCommandParse(vshControl *ctl, vshCommandParser *parser) continue; } else { get_data: + /* Special case 'help' to ignore spurious data */ if (!(opt = vshCmddefGetData(cmd, &opts_need_arg, - &opts_seen))) { + &opts_seen)) && + STRNEQ(cmd->name, "help")) { vshError(ctl, _("unexpected data '%s'"), tkdata); goto syntaxError; } -- 1.7.11.2

On 08/13/2012 08:28 AM, Cole Robinson wrote:
Similar to the previous patch, prepending 'help' to a partial command string doesn't cut us any slack.
$ virsh help pool-define-as --name foo --type dir error: command 'help' doesn't support option --name
This patch adds a few hacks to make 'help' ignore everything after the first data bit, so the above command shows help output for pool-define-as. --- tools/virsh.c | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-)
Also passed my testing. ACK. -- Eric Blake eblake@redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org

On Mon, Aug 13, 2012 at 9:28 AM, Cole Robinson <crobinso@redhat.com> wrote:
Similar to the previous patch, prepending 'help' to a partial command string doesn't cut us any slack.
$ virsh help pool-define-as --name foo --type dir error: command 'help' doesn't support option --name
This patch adds a few hacks to make 'help' ignore everything after the first data bit, so the above command shows help output for pool-define-as. --- tools/virsh.c | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-)
diff --git a/tools/virsh.c b/tools/virsh.c index 324f789..ab8d6dc 100644 --- a/tools/virsh.c +++ b/tools/virsh.c @@ -1219,8 +1219,10 @@ vshCmddefGetOption(vshControl *ctl, const vshCmdDef *cmd, const char *name, } }
- vshError(ctl, _("command '%s' doesn't support option --%s"), - cmd->name, name); + if (STRNEQ(cmd->name, "help")) { + vshError(ctl, _("command '%s' doesn't support option --%s"), + cmd->name, name); + } return NULL; }
@@ -1987,9 +1989,12 @@ vshCommandParse(vshControl *ctl, vshCommandParser *parser) *optstr = '\0'; /* convert the '=' to '\0' */ optstr = vshStrdup(ctl, optstr + 1); } + /* Special case 'help' to ignore all spurious options */ if (!(opt = vshCmddefGetOption(ctl, cmd, tkdata + 2, &opts_seen, &opt_index))) { VIR_FREE(optstr); + if (STREQ(cmd->name, "help")) + continue; goto syntaxError; } VIR_FREE(tkdata); @@ -2027,8 +2032,10 @@ vshCommandParse(vshControl *ctl, vshCommandParser *parser) continue; } else { get_data: + /* Special case 'help' to ignore spurious data */ if (!(opt = vshCmddefGetData(cmd, &opts_need_arg, - &opts_seen))) { + &opts_seen)) && + STRNEQ(cmd->name, "help")) { vshError(ctl, _("unexpected data '%s'"), tkdata); goto syntaxError; } -- 1.7.11.2
Worked for me when I tested it with the previous patches so ACK. -- Doug Goldstein

On 08/13/2012 02:53 PM, Doug Goldstein wrote:
On Mon, Aug 13, 2012 at 9:28 AM, Cole Robinson <crobinso@redhat.com> wrote:
Similar to the previous patch, prepending 'help' to a partial command string doesn't cut us any slack.
$ virsh help pool-define-as --name foo --type dir error: command 'help' doesn't support option --name
This patch adds a few hacks to make 'help' ignore everything after the first data bit, so the above command shows help output for pool-define-as. --- tools/virsh.c | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-)
diff --git a/tools/virsh.c b/tools/virsh.c index 324f789..ab8d6dc 100644 --- a/tools/virsh.c +++ b/tools/virsh.c @@ -1219,8 +1219,10 @@ vshCmddefGetOption(vshControl *ctl, const vshCmdDef *cmd, const char *name, } }
- vshError(ctl, _("command '%s' doesn't support option --%s"), - cmd->name, name); + if (STRNEQ(cmd->name, "help")) { + vshError(ctl, _("command '%s' doesn't support option --%s"), + cmd->name, name); + } return NULL; }
@@ -1987,9 +1989,12 @@ vshCommandParse(vshControl *ctl, vshCommandParser *parser) *optstr = '\0'; /* convert the '=' to '\0' */ optstr = vshStrdup(ctl, optstr + 1); } + /* Special case 'help' to ignore all spurious options */ if (!(opt = vshCmddefGetOption(ctl, cmd, tkdata + 2, &opts_seen, &opt_index))) { VIR_FREE(optstr); + if (STREQ(cmd->name, "help")) + continue; goto syntaxError; } VIR_FREE(tkdata); @@ -2027,8 +2032,10 @@ vshCommandParse(vshControl *ctl, vshCommandParser *parser) continue; } else { get_data: + /* Special case 'help' to ignore spurious data */ if (!(opt = vshCmddefGetData(cmd, &opts_need_arg, - &opts_seen))) { + &opts_seen)) && + STRNEQ(cmd->name, "help")) { vshError(ctl, _("unexpected data '%s'"), tkdata); goto syntaxError; } -- 1.7.11.2
Worked for me when I tested it with the previous patches so ACK.
Thanks guys, pushed now. - Cole

On Mon, Aug 13, 2012 at 9:28 AM, Cole Robinson <crobinso@redhat.com> wrote:
For some reason I only get this after applying subsequent upcoming patches that touch virsh, but don't seem to actually cause the warning.
virsh.c: In function ‘vshCommandParse’: virsh.c:2014:46: error: ‘opt_index’ may be used uninitialized in this function [-Werror=maybe-uninitialized] cc1: all warnings being treated as errors --- tools/virsh.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/tools/virsh.c b/tools/virsh.c index b95a008..64e2e18 100644 --- a/tools/virsh.c +++ b/tools/virsh.c @@ -1975,7 +1975,7 @@ vshCommandParse(vshControl *ctl, vshCommandParser *parser) } else if (tkdata[0] == '-' && tkdata[1] == '-' && c_isalnum(tkdata[2])) { char *optstr = strchr(tkdata + 2, '='); - int opt_index; + int opt_index = 0;
if (optstr) { *optstr = '\0'; /* convert the '=' to '\0' */ -- 1.7.11.2
ACK. Looks good. -- Doug Goldstein
participants (3)
-
Cole Robinson
-
Doug Goldstein
-
Eric Blake