On readline completion vshReadlineCompletion() is called which
does nothing more than calling rl_completion_matches() with
vshReadlineParse() as a callback. This means, that
vshReadlineParse() is called repeatedly, each time returning next
completion candidate, until it returns NULL which is interpreted
as the end of the list of candidates. The function takes two
parameters: @text which is a portion of input line around cursor
when TAB was pressed, and @state. The @state is an integer that
is zero on the very first call and non-zero on each subsequent
call (in fact, readline does @state++ on each call). Anyway, the
idea is that the callback gets the whole list of candidates on
@state == 0 and returns one candidate at each call. And this is
what vshReadlineParse() is doing but some variables (@parital,
@cmd and @opt) are really used only in the @state == 0 case but
declared for whole function. We can limit their scope by
declaring them inside the @state == 0 body which also means that
they don't have to be static anymore.

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

diff --git a/tools/vsh.c b/tools/vsh.c
index 9856088126..ba6299aae4 100644
--- a/tools/vsh.c
+++ b/tools/vsh.c
@@ -2695,18 +2695,20 @@ vshCompleterFilter(char ***list,
 static char *
 vshReadlineParse(const char *text, int state)
 {
-    static vshCmd *partial;
     static char **list;
     static size_t list_index;
-    const vshCmdDef *cmd = NULL;
-    const vshCmdOptDef *opt = NULL;
     char *ret = NULL;
 
+    /* Readline calls this function until NULL is returned. On
+     * the very first call @state is zero which means we should
+     * initialize those static variables above. On subsequent
+     * calls @state is non zero. */
     if (!state) {
+        vshCmd *partial = NULL;
+        const vshCmdDef *cmd = NULL;
+        const vshCmdOptDef *opt = NULL;
         char *buf = g_strdup(rl_line_buffer);
 
-        vshCommandFree(partial);
-        partial = NULL;
         g_strfreev(list);
         list = NULL;
         list_index = 0;
@@ -2734,9 +2736,7 @@ vshReadlineParse(const char *text, int state)
         }
 
         opt = vshReadlineCommandFindOpt(partial, text);
-    }
 
-    if (!list) {
         if (!cmd) {
             list = vshReadlineCommandGenerator(text);
         } else {
@@ -2759,10 +2759,12 @@ vshReadlineParse(const char *text, int state)
                     (vshCompleterFilter(&completer_list, text) < 0 ||
                      virStringListMerge(&list, &completer_list) < 0)) {
                     g_strfreev(completer_list);
+                    vshCommandFree(partial);
                     goto cleanup;
                 }
             }
         }
+        vshCommandFree(partial);
     }
 
     if (list) {
@@ -2780,15 +2782,12 @@ vshReadlineParse(const char *text, int state)
 
  cleanup:
     if (!ret) {
-        vshCommandFree(partial);
-        partial = NULL;
         g_strfreev(list);
         list = NULL;
         list_index = 0;
     }
 
     return ret;
-
 }
 
 static char **
-- 
2.26.2

On a Tuesday in 2021, Michal Privoznik wrote:
>On readline completion vshReadlineCompletion() is called which
>does nothing more than calling rl_completion_matches() with
>vshReadlineParse() as a callback. This means, that
>vshReadlineParse() is called repeatedly, each time returning next
>completion candidate, until it returns NULL which is interpreted
>as the end of the list of candidates. The function takes two
>parameters: @text which is a portion of input line around cursor
>when TAB was pressed, and @state. The @state is an integer that
>is zero on the very first call and non-zero on each subsequent
>call (in fact, readline does @state++ on each call). Anyway, the
>idea is that the callback gets the whole list of candidates on
>@state == 0 and returns one candidate at each call. And this is
>what vshReadlineParse() is doing but some variables (@parital,

* partial

Also, the commit message could use some newlines.

Jano

>@cmd and @opt) are really used only in the @state == 0 case but
>declared for whole function. We can limit their scope by
>declaring them inside the @state == 0 body which also means that
>they don't have to be static anymore.
>
>Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
>---
> tools/vsh.c | 19 +++++++++----------
> 1 file changed, 9 insertions(+), 10 deletions(-)
>