None of the clients use the 'command set' approach and other pieces of
code such as the command validator already assume that command groups
are in use. Remove the unused 'command set' stuff.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
---
 tools/virsh.c      |  2 +-
 tools/virt-admin.c |  2 +-
 tools/vsh.c        | 38 +++++++-------------------------------
 tools/vsh.h        |  2 +-
 4 files changed, 10 insertions(+), 34 deletions(-)

diff --git a/tools/virsh.c b/tools/virsh.c
index 95ff63baeb..18cd279aba 100644
--- a/tools/virsh.c
+++ b/tools/virsh.c
@@ -878,7 +878,7 @@ main(int argc, char **argv)

     virFileActivateDirOverrideForProg(argv[0]);

-    if (!vshInit(ctl, cmdGroups, NULL))
+    if (!vshInit(ctl, cmdGroups))
         exit(EXIT_FAILURE);

     if (!virshParseArgv(ctl, argc, argv) ||
diff --git a/tools/virt-admin.c b/tools/virt-admin.c
index aaf6edb9a9..f551b33c4b 100644
--- a/tools/virt-admin.c
+++ b/tools/virt-admin.c
@@ -1594,7 +1594,7 @@ main(int argc, char **argv)

     virFileActivateDirOverrideForProg(argv[0]);

-    if (!vshInit(ctl, cmdGroups, NULL))
+    if (!vshInit(ctl, cmdGroups))
         exit(EXIT_FAILURE);

     if (!vshAdmParseArgv(ctl, argc, argv) ||
diff --git a/tools/vsh.c b/tools/vsh.c
index 65deaa77e8..0c41e9cfe8 100644
--- a/tools/vsh.c
+++ b/tools/vsh.c
@@ -56,7 +56,6 @@ vshControl *autoCompleteOpaque;
  * and only relies on static data accessible from the user-side callback
  */
 const vshCmdGrp *cmdGroups;
-const vshCmdDef *cmdSet;


 double
@@ -572,8 +571,8 @@ vshCommandCheckOpts(vshControl *ctl, const vshCmd *cmd, uint64_t opts_required,
     return -1;
 }

-static const vshCmdDef *
-vshCmdDefSearchGrp(const char *cmdname)
+const vshCmdDef *
+vshCmddefSearch(const char *cmdname)
 {
     const vshCmdGrp *g;
     const vshCmdDef *c;
@@ -588,28 +587,6 @@ vshCmdDefSearchGrp(const char *cmdname)
     return NULL;
 }

-static const vshCmdDef *
-vshCmdDefSearchSet(const char *cmdname)
-{
-    const vshCmdDef *s;
-
-    for (s = cmdSet; s->name; s++) {
-        if (STREQ(s->name, cmdname))
-            return s;
-        }
-
-    return NULL;
-}
-
-const vshCmdDef *
-vshCmddefSearch(const char *cmdname)
-{
-    if (cmdGroups)
-        return vshCmdDefSearchGrp(cmdname);
-    else
-        return vshCmdDefSearchSet(cmdname);
-}
-
 const vshCmdGrp *
 vshCmdGrpSearch(const char *grpname)
 {
@@ -3012,20 +2989,19 @@ vshInitDebug(vshControl *ctl)
  * Initialize global data
  */
 bool
-vshInit(vshControl *ctl, const vshCmdGrp *groups, const vshCmdDef *set)
+vshInit(vshControl *ctl, const vshCmdGrp *groups)
 {
     if (!ctl->hooks) {
         vshError(ctl, "%s", _("client hooks cannot be NULL"));
         return false;
     }

-    if (!groups && !set) {
-        vshError(ctl, "%s", _("command groups and command set cannot both be NULL"));
+    if (!groups) {
+        vshError(ctl, "%s", _("command groups must be non-NULL"));
         return false;
     }

     cmdGroups = groups;
-    cmdSet = set;

     if (vshInitDebug(ctl) < 0 ||
         (ctl->imode && vshReadlineInit(ctl) < 0))
@@ -3037,8 +3013,8 @@ vshInit(vshControl *ctl, const vshCmdGrp *groups, const vshCmdDef *set)
 bool
 vshInitReload(vshControl *ctl)
 {
-    if (!cmdGroups && !cmdSet) {
-        vshError(ctl, "%s", _("command groups and command are both NULL run vshInit before reloading"));
+    if (!cmdGroups) {
+        vshError(ctl, "%s", _("command groups is NULL run vshInit before reloading"));
         return false;
     }

diff --git a/tools/vsh.h b/tools/vsh.h
index 2a1be29b1c..17d7f08dc9 100644
--- a/tools/vsh.h
+++ b/tools/vsh.h
@@ -310,7 +310,7 @@ void vshPrint(vshControl *ctl, const char *format, ...)
     G_GNUC_PRINTF(2, 3);
 void vshPrintExtra(vshControl *ctl, const char *format, ...)
     G_GNUC_PRINTF(2, 3);
-bool vshInit(vshControl *ctl, const vshCmdGrp *groups, const vshCmdDef *set);
+bool vshInit(vshControl *ctl, const vshCmdGrp *groups);
 bool vshInitReload(vshControl *ctl);
 void vshDeinit(vshControl *ctl);
 void vshDebug(vshControl *ctl, int level, const char *format, ...)
-- 
2.44.0
_______________________________________________
Devel mailing list -- devel@lists.libvirt.org
To unsubscribe send an email to devel-leave@lists.libvirt.org

On Mon, Mar 11, 2024 at 12:30:38 +0100, Peter Krempa wrote:
> None of the clients use the 'command set' approach and other pieces of
> code such as the command validator already assume that command groups
> are in use. Remove the unused 'command set' stuff.
> 
> Signed-off-by: Peter Krempa <pkrempa@redhat.com>
> ---
>  tools/virsh.c      |  2 +-
>  tools/virt-admin.c |  2 +-
>  tools/vsh.c        | 38 +++++++-------------------------------
>  tools/vsh.h        |  2 +-
>  4 files changed, 10 insertions(+), 34 deletions(-)

The compiler on mingw started thinking after this patch that it sees a
potential NULL dereference.

To avoid that (while it's impossible as we check the assumptions which
would make the command lookup function return NULL) I'll be squashing in
the following to silence it:

diff --git a/tools/vsh.c b/tools/vsh.c
index cbd0d88078..7a8e9f7f8c 100644
--- a/tools/vsh.c
+++ b/tools/vsh.c
@@ -1402,7 +1402,11 @@ vshCommandParse(vshControl *ctl, vshCommandParser *parser, vshCmd **partial)
                 if (cmd->flags & VSH_CMD_FLAG_ALIAS) {
                     VIR_FREE(tkdata);
                     tkdata = g_strdup(cmd->alias);
-                    cmd = vshCmddefSearch(tkdata);
+                    if (!(cmd = vshCmddefSearch(tkdata))) {
+                        /* self-test ensures that the alias exists */
+                        vshError(ctl, _("unknown command: '%1$s'"), tkdata);
+                        goto syntaxError;
+                    }
                 }

                 vshCmddefOptParse(cmd, &opts_need_arg, &opts_required);
@@ -1527,7 +1531,10 @@ vshCommandParse(vshControl *ctl, vshCommandParser *parser, vshCmd **partial)
                 if (STRNEQ(tmpopt->def->name, "help"))
                     continue;

-                help = vshCmddefSearch("help");
+                /* the self-test code ensures that help exists */
+                if (!(help = vshCmddefSearch("help")))
+                    break;
+
                 vshCommandOptFree(first);
                 first = g_new0(vshCmdOpt, 1);
                 first->def = help->opts;
@@ -3132,6 +3139,9 @@ cmdHelp(vshControl *ctl, const vshCmd *cmd)
     if ((def = vshCmddefSearch(name))) {
         if (def->flags & VSH_CMD_FLAG_ALIAS)
             def = vshCmddefSearch(def->alias);
+    }
+
+    if (def) {
         return vshCmddefHelp(def);
     } else if ((grp = vshCmdGrpSearch(name))) {
         return vshCmdGrpHelp(ctl, grp);
_______________________________________________
Devel mailing list -- devel@lists.libvirt.org
To unsubscribe send an email to devel-leave@lists.libvirt.org