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

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

[Dongli Zhang]

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;
         }
@@ -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':
-- 
2.34.1

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

[Philippe Mathieu-Daudé]

On 7/2/23 05:52, Dongli Zhang wrote:
> 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;
>           }
> @@ -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':

Tested-by: Philippe Mathieu-Daudé <philmd@linaro.org>

Thanks!

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!

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

[Thomas Huth]

On 07/02/2023 05.52, Dongli Zhang wrote:
> Subject:
> [PATCH 1/1] readline: fix hmp completion issue
> From:
> Dongli Zhang <dongli.zhang@oracle.com>
> Date:
> 07/02/2023, 05.52
> 
> To:
> qemu-devel@nongnu.org
> CC:
> dgilbert@redhat.com, joe.jin@oracle.com
> 
> 
> 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;
>           }
> @@ -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':
> -- 2.34.1
> 

Thanks, this fixes the completion for me, too:

Tested-by: Thomas Huth <thuth@redhat.com>