9:22 a.m.
The alias doesn't have any definition rather than a reference for the aliased
command. Therefore when printing help, we have to print help for the aliased
command, otherwise our faith is going to be met with SIGSEGV.
Erik Skultety (2):
vsh: Drop redundant definition searches from vshCmd{def,Grp}Help
vsh: Cmd aliases lookups should return results for the aliased command
tools/vsh.c | 45 +++++++++++++++++----------------------------
tools/vsh.h | 4 ++--
2 files changed, 19 insertions(+), 30 deletions(-)
--
2.13.6
9:22 a.m.
New subject: [libvirt] [PATCH 1/2] vsh: Drop redundant definition searches from vshCmd{def, Grp}Help
These helpers are called from a single place only - cmdHelp wrapper and
just before the wrapper invokes the helpers, it performs the search,
either for command group or for the command itself, except the result is
discarded and the helper therefore needs to do it again. Drop this
inefficient handling and pass the @def structure rather than a name,
thus preventing the helper from needing to perform the search again.
Signed-off-by: Erik Skultety <eskultet(a)redhat.com>
---
tools/vsh.c | 43 +++++++++++++++----------------------------
tools/vsh.h | 4 ++--
2 files changed, 17 insertions(+), 30 deletions(-)
diff --git a/tools/vsh.c b/tools/vsh.c
index 058df7ef6..761d2ec3a 100644
--- a/tools/vsh.c
+++ b/tools/vsh.c
@@ -629,44 +629,32 @@ vshCmdGrpSearch(const char *grpname)
}
bool
-vshCmdGrpHelp(vshControl *ctl, const char *grpname)
+vshCmdGrpHelp(vshControl *ctl, const vshCmdGrp *grp)
{
- const vshCmdGrp *grp = vshCmdGrpSearch(grpname);
const vshCmdDef *cmd = NULL;
- if (!grp) {
- vshError(ctl, _("command group '%s' doesn't exist"),
grpname);
- return false;
- } else {
- vshPrint(ctl, _(" %s (help keyword '%s'):\n"), grp->name,
- grp->keyword);
+ vshPrint(ctl, _(" %s (help keyword '%s'):\n"), grp->name,
+ grp->keyword);
- for (cmd = grp->commands; cmd->name; cmd++) {
- if (cmd->flags & VSH_CMD_FLAG_ALIAS)
- continue;
- vshPrint(ctl, " %-30s %s\n", cmd->name,
- _(vshCmddefGetInfo(cmd, "help")));
- }
+ for (cmd = grp->commands; cmd->name; cmd++) {
+ if (cmd->flags & VSH_CMD_FLAG_ALIAS)
+ continue;
+ vshPrint(ctl, " %-30s %s\n", cmd->name,
+ _(vshCmddefGetInfo(cmd, "help")));
}
return true;
}
bool
-vshCmddefHelp(vshControl *ctl, const char *cmdname)
+vshCmddefHelp(vshControl *ctl, const vshCmdDef *def)
{
- const vshCmdDef *def = vshCmddefSearch(cmdname);
const char *desc = NULL;
char buf[256];
uint64_t opts_need_arg;
uint64_t opts_required;
bool shortopt = false; /* true if 'arg' works instead of '--opt arg'
*/
- if (!def) {
- vshError(ctl, _("command '%s' doesn't exist"), cmdname);
- return false;
- }
-
if (vshCmddefOptParse(def, &opts_need_arg, &opts_required)) {
vshError(ctl, _("internal error: bad options in command:
'%s'"),
def->name);
@@ -3181,12 +3169,11 @@ const vshCmdInfo info_help[] = {
bool
cmdHelp(vshControl *ctl, const vshCmd *cmd)
{
+ const vshCmdDef *def = NULL;
+ const vshCmdGrp *grp = NULL;
const char *name = NULL;
if (vshCommandOptStringQuiet(ctl, cmd, "command", &name) <= 0) {
- const vshCmdGrp *grp;
- const vshCmdDef *def;
-
vshPrint(ctl, "%s", _("Grouped commands:\n\n"));
for (grp = cmdGroups; grp->name; grp++) {
@@ -3206,10 +3193,10 @@ cmdHelp(vshControl *ctl, const vshCmd *cmd)
return true;
}
- if (vshCmddefSearch(name)) {
- return vshCmddefHelp(ctl, name);
- } else if (vshCmdGrpSearch(name)) {
- return vshCmdGrpHelp(ctl, name);
+ if ((def = vshCmddefSearch(name))) {
+ return vshCmddefHelp(ctl, def);
+ } else if ((grp = vshCmdGrpSearch(name))) {
+ return vshCmdGrpHelp(ctl, grp);
} else {
vshError(ctl, _("command or command group '%s' doesn't
exist"), name);
return false;
diff --git a/tools/vsh.h b/tools/vsh.h
index 6894700d9..694476471 100644
--- a/tools/vsh.h
+++ b/tools/vsh.h
@@ -257,9 +257,9 @@ void vshCloseLogFile(vshControl *ctl);
const char *vshCmddefGetInfo(const vshCmdDef *cmd, const char *info);
const vshCmdDef *vshCmddefSearch(const char *cmdname);
-bool vshCmddefHelp(vshControl *ctl, const char *name);
+bool vshCmddefHelp(vshControl *ctl, const vshCmdDef *def);
const vshCmdGrp *vshCmdGrpSearch(const char *grpname);
-bool vshCmdGrpHelp(vshControl *ctl, const char *name);
+bool vshCmdGrpHelp(vshControl *ctl, const vshCmdGrp *grp);
int vshCommandOptInt(vshControl *ctl, const vshCmd *cmd,
const char *name, int *value)
--
2.13.6
9:22 a.m.
New subject: [libvirt] [PATCH 2/2] vsh: Cmd aliases lookups should return results for the aliased command
Unfortunately, we have a number of aliases in virsh and even though
these are not visible any more, we have to support them. The problem is
that when trying to print help for the alias, we get SIGSEGV because
there isn't any @def structure anymore and we need to query the command
being aliased instead.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1538570
Signed-off-by: Erik Skultety <eskultet(a)redhat.com>
---
tools/vsh.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/tools/vsh.c b/tools/vsh.c
index 761d2ec3a..37c292a03 100644
--- a/tools/vsh.c
+++ b/tools/vsh.c
@@ -3194,6 +3194,8 @@ cmdHelp(vshControl *ctl, const vshCmd *cmd)
}
if ((def = vshCmddefSearch(name))) {
+ if (def->flags & VSH_CMD_FLAG_ALIAS)
+ def = vshCmddefSearch(def->alias);
return vshCmddefHelp(ctl, def);
} else if ((grp = vshCmdGrpSearch(name))) {
return vshCmdGrpHelp(ctl, grp);
--
2.13.6
9:49 a.m.
On 01/25/2018 04:22 PM, Erik Skultety wrote:
The alias doesn't have any definition rather than a reference for
the aliased
command. Therefore when printing help, we have to print help for the aliased
command, otherwise our faith is going to be met with SIGSEGV.
Erik Skultety (2):
vsh: Drop redundant definition searches from vshCmd{def,Grp}Help
vsh: Cmd aliases lookups should return results for the aliased command
tools/vsh.c | 45 +++++++++++++++++----------------------------
tools/vsh.h | 4 ++--
2 files changed, 19 insertions(+), 30 deletions(-)
ACK series
Michal
10:11 a.m.
On Thu, Jan 25, 2018 at 04:49:38PM +0100, Michal Privoznik wrote:
On 01/25/2018 04:22 PM, Erik Skultety wrote:
> The alias doesn't have any definition rather than a reference for the aliased
> command. Therefore when printing help, we have to print help for the aliased
> command, otherwise our faith is going to be met with SIGSEGV.
>
> Erik Skultety (2):
> vsh: Drop redundant definition searches from vshCmd{def,Grp}Help
> vsh: Cmd aliases lookups should return results for the aliased command
>
> tools/vsh.c | 45 +++++++++++++++++----------------------------
> tools/vsh.h | 4 ++--
> 2 files changed, 19 insertions(+), 30 deletions(-)
>
ACK series
Pushed, thanks.
Erik
2496
days inactive
2496
days old
4 comments
2 participants
participants (2)
-
Erik Skultety -
Michal Privoznik