The current state of art is as follows:

 1) vshReadlineOptionsGenerator() generate all possible --options
 for given command, and then
 2) vshReadlineOptionsPrune() clears out already provided ones
 from the list.

Not only this brings needless memory complexity it is also not
trivial to get right. We can switch to easier approach: just
don't add already specified --options in the first step.

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
---
 tools/vsh.c | 80 +++++++++++++++----------------------------------------------
 1 file changed, 19 insertions(+), 61 deletions(-)

diff --git a/tools/vsh.c b/tools/vsh.c
index 4426c08d6..a99d47bfc 100644
--- a/tools/vsh.c
+++ b/tools/vsh.c
@@ -2663,7 +2663,9 @@ vshReadlineCommandGenerator(const char *text)
 }
 
 static char **
-vshReadlineOptionsGenerator(const char *text, const vshCmdDef *cmd)
+vshReadlineOptionsGenerator(const char *text,
+                            const vshCmdDef *cmd,
+                            vshCmd *last)
 {
     size_t list_index = 0;
     size_t len = strlen(text);
@@ -2678,6 +2680,8 @@ vshReadlineOptionsGenerator(const char *text, const vshCmdDef *cmd)
         return NULL;
 
     while ((name = cmd->opts[list_index].name)) {
+        bool exists = false;
+        vshCmdOpt *opt =  last->opts;
         size_t name_len;
 
         list_index++;
@@ -2692,6 +2696,18 @@ vshReadlineOptionsGenerator(const char *text, const vshCmdDef *cmd)
             return NULL;
         }
 
+        while (opt) {
+            if (STREQ(opt->def->name, name)) {
+                exists = true;
+                break;
+            }
+
+            opt = opt->next;
+        }
+
+        if (exists)
+            continue;
+
         if (VIR_REALLOC_N(ret, ret_size + 2) < 0) {
             virStringListFree(ret);
             return NULL;
@@ -2772,60 +2788,6 @@ vshCompleterFilter(char ***list,
 }
 
 
-static int
-vshReadlineOptionsPrune(char ***list,
-                        vshCmd *last)
-{
-    char **newList = NULL;
-    size_t newList_len = 0;
-    size_t list_len;
-    size_t i;
-
-    if (!list || !*list)
-        return -1;
-
-    if (!last->opts)
-        return 0;
-
-    list_len = virStringListLength((const char **) *list);
-
-    if (VIR_ALLOC_N(newList, list_len + 1) < 0)
-        return -1;
-
-    for (i = 0; i < list_len; i++) {
-        const char *list_opt = STRSKIP((*list)[i], "--");
-        bool exist = false;
-        vshCmdOpt *opt =  last->opts;
-
-        /* Should never happen (TM) */
-        if (!list_opt)
-            return -1;
-
-        while (opt) {
-            if (STREQ(opt->def->name, list_opt)) {
-                exist = true;
-                break;
-            }
-
-            opt = opt->next;
-        }
-
-        if (exist) {
-            VIR_FREE((*list)[i]);
-            continue;
-        }
-
-        VIR_STEAL_PTR(newList[newList_len], (*list)[i]);
-        newList_len++;
-    }
-
-    ignore_value(VIR_REALLOC_N_QUIET(newList, newList_len + 1));
-    VIR_FREE(*list);
-    *list = newList;
-    return 0;
-}
-
-
 static char *
 vshReadlineParse(const char *text, int state)
 {
@@ -2874,12 +2836,8 @@ vshReadlineParse(const char *text, int state)
         if (!cmd) {
             list = vshReadlineCommandGenerator(text);
         } else {
-            if (!opt || (opt->type != VSH_OT_DATA && opt->type != VSH_OT_STRING)) {
-                list = vshReadlineOptionsGenerator(text, cmd);
-
-                if (vshReadlineOptionsPrune(&list, partial) < 0)
-                    goto cleanup;
-            }
+            if (!opt || (opt->type != VSH_OT_DATA && opt->type != VSH_OT_STRING))
+                list = vshReadlineOptionsGenerator(text, cmd, partial);
 
             if (opt && opt->completer) {
                 char **completer_list = opt->completer(autoCompleteOpaque,
-- 
2.13.6

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

On Mon, Jan 15, 2018 at 10:34:29AM +0100, Michal Privoznik wrote:
> The current state of art is as follows:
>
>  1) vshReadlineOptionsGenerator() generate all possible --options
>  for given command, and then
>  2) vshReadlineOptionsPrune() clears out already provided ones
>  from the list.
>
> Not only this brings needless memory complexity it is also not
> trivial to get right. We can switch to easier approach: just
> don't add already specified --options in the first step.
>
> Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
> ---
Reviewed-by: Erik Skultety <eskultet@redhat.com>

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list