Re: [PATCH 1/1] readline: fix hmp completion issue

[Markus Armbruster]

Dongli Zhang <dongli.zhang@oracle.com> writes:

> The auto completion does not work in some cases.
>
> Case 1.
>
> 1. (qemu) info reg
> 2. Press 'Tab'.
> 3. It does not auto complete.
>
> Case 2.
>
> 1. (qemu) block_resize flo
> 2. Press 'Tab'.
> 3. It does not auto complete 'floppy0'.
>
> Since the readline_add_completion_of() may add any completion when
> strlen(pfx) is zero, we remove the check with (name[0] == '\0') because
> strlen() always returns zero in that case.
>
> Fixes: 52f50b1e9f8f ("readline: Extract readline_add_completion_of() from 
> monitor")
> Cc: Joe Jin <joe.jin@oracle.com>
> Signed-off-by: Dongli Zhang <dongli.zhang@oracle.com>
> ---
>  monitor/hmp.c | 8 ++------
>  1 file changed, 2 insertions(+), 6 deletions(-)
>
> diff --git a/monitor/hmp.c b/monitor/hmp.c
> index 2aa85d3982..fee410362f 100644
> --- a/monitor/hmp.c
> +++ b/monitor/hmp.c
> @@ -1189,9 +1189,7 @@ static void cmd_completion(MonitorHMP *mon, const char 
> *name, const char *list)
>          }
>          memcpy(cmd, pstart, len);
>          cmd[len] = '\0';
> -        if (name[0] == '\0') {
> -            readline_add_completion_of(mon->rs, name, cmd);
> -        }
> +        readline_add_completion_of(mon->rs, name, cmd);
>          if (*p == '\0') {
>              break;
>          }

Yes, this fixes a silly logic error in my patch.

Before my patch:

        if (name[0] == '\0' || !strncmp(name, cmd, strlen(name))) {
            readline_add_completion(mon->rs, cmd);
        }

The left operand of || implies the right operand: it's true when name is
"", and strncmp("", ..., 0) is 0.  Therefore, this could simply be

        if (!strncmp(name, cmd, strlen(name))) {
            readline_add_completion(mon->rs, cmd);
        }

My patch combined with yours factors this out.

> @@ -1335,9 +1333,7 @@ static void monitor_find_completion_by_table(MonitorHMP 
> *mon,
>              /* block device name completion */
>              readline_set_completion_index(mon->rs, strlen(str));
>              while ((blk = blk_next(blk)) != NULL) {
> -                if (str[0] == '\0') {
> -                    readline_add_completion_of(mon->rs, str, blk_name(blk));
> -                }
> +                readline_add_completion_of(mon->rs, str, blk_name(blk));
>              }
>              break;
>          case 's':

Likewise.

Reviewed-by: Markus Armbruster <armbru@redhat.com>

I'll take this through my tree.  Thanks!