diff mbox

[v2] monitor: report entirety of hmp command on error

Message ID ee680f5e-ac9a-479d-f65e-9f8ae9cfe5d4@linux.ibm.com (mailing list archive)
State New, archived
Headers show

Commit Message

Collin Walling May 7, 2018, 2:30 p.m. UTC
When a user incorrectly provides an hmp command, an error response will be
printed that prompts the user to try "help <command name>". However, when
the command contains multiple parts e.g. "info uuid xyz", only the last
whitespace delimited string will be reported (in this example "info" will
be dropped and the message will read "Try "help uuid" for more information",
which is incorrect).

Let's correct this by capturing the entirety of the command from the command
line -- excluding any extraneous characters.

Reported-by: Mikhail Fokin <fokin@de.ibm.com>
Signed-off-by: Collin Walling <walling@linux.ibm.com>
---
 monitor.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

Comments

Eric Blake May 7, 2018, 4:44 p.m. UTC | #1
On 05/07/2018 09:30 AM, Collin Walling wrote:
> When a user incorrectly provides an hmp command, an error response will be
> printed that prompts the user to try "help <command name>". However, when
> the command contains multiple parts e.g. "info uuid xyz", only the last
> whitespace delimited string will be reported (in this example "info" will
> be dropped and the message will read "Try "help uuid" for more information",
> which is incorrect).
> 
> Let's correct this by capturing the entirety of the command from the command
> line -- excluding any extraneous characters.
> 

It's better to post a v2 patch as a new top-level thread instead of 
in-reply to an earlier version, as some of our automated tooling is more 
likely to see it.

> Reported-by: Mikhail Fokin <fokin@de.ibm.com>
> Signed-off-by: Collin Walling <walling@linux.ibm.com>
> ---
>   monitor.c | 8 ++++++--
>   1 file changed, 6 insertions(+), 2 deletions(-)

Reviewed-by: Eric Blake <eblake@redhat.com>
Collin Walling May 7, 2018, 5:10 p.m. UTC | #2
On 05/07/2018 12:44 PM, Eric Blake wrote:
> On 05/07/2018 09:30 AM, Collin Walling wrote:
>> When a user incorrectly provides an hmp command, an error response will be
>> printed that prompts the user to try "help <command name>". However, when
>> the command contains multiple parts e.g. "info uuid xyz", only the last
>> whitespace delimited string will be reported (in this example "info" will
>> be dropped and the message will read "Try "help uuid" for more information",
>> which is incorrect).
>>
>> Let's correct this by capturing the entirety of the command from the command
>> line -- excluding any extraneous characters.
>>
> 
> It's better to post a v2 patch as a new top-level thread instead of in-reply to an earlier version, as some of our automated tooling is more likely to see it.
> 
>> Reported-by: Mikhail Fokin <fokin@de.ibm.com>
>> Signed-off-by: Collin Walling <walling@linux.ibm.com>
>> ---
>>   monitor.c | 8 ++++++--
>>   1 file changed, 6 insertions(+), 2 deletions(-)
> 
> Reviewed-by: Eric Blake <eblake@redhat.com>
> 

Noted. And thank you for the r-b.
Markus Armbruster May 24, 2018, 2:16 p.m. UTC | #3
David, looks like your turf.

Collin Walling <walling@linux.ibm.com> writes:

> When a user incorrectly provides an hmp command, an error response will be
> printed that prompts the user to try "help <command name>". However, when
> the command contains multiple parts e.g. "info uuid xyz", only the last
> whitespace delimited string will be reported (in this example "info" will
> be dropped and the message will read "Try "help uuid" for more information",
> which is incorrect).
>
> Let's correct this by capturing the entirety of the command from the command
> line -- excluding any extraneous characters.
>
> Reported-by: Mikhail Fokin <fokin@de.ibm.com>
> Signed-off-by: Collin Walling <walling@linux.ibm.com>
> ---
>  monitor.c | 8 ++++++--
>  1 file changed, 6 insertions(+), 2 deletions(-)
>
> diff --git a/monitor.c b/monitor.c
> index 39f8ee1..38736b3 100644
> --- a/monitor.c
> +++ b/monitor.c
> @@ -3371,6 +3371,7 @@ static void handle_hmp_command(Monitor *mon, const char *cmdline)
>  {
>      QDict *qdict;
>      const mon_cmd_t *cmd;
> +    const char *cmd_start = cmdline;
>  
>      trace_handle_hmp_command(mon, cmdline);
>  
> @@ -3381,8 +3382,11 @@ static void handle_hmp_command(Monitor *mon, const char *cmdline)
>  
>      qdict = monitor_parse_arguments(mon, &cmdline, cmd);
>      if (!qdict) {
> -        monitor_printf(mon, "Try \"help %s\" for more information\n",
> -                       cmd->name);
> +        while (cmdline > cmd_start && qemu_isspace(cmdline[-1])) {
> +            cmdline--;
> +        }
> +        monitor_printf(mon, "Try \"help %.*s\" for more information\n",
> +                       (int)(cmdline - cmd_start), cmd_start);
>          return;
>      }
Dr. David Alan Gilbert May 30, 2018, 10:15 a.m. UTC | #4
* Markus Armbruster (armbru@redhat.com) wrote:
> David, looks like your turf.

Yep, I've got it on my list to take.

Dave

> Collin Walling <walling@linux.ibm.com> writes:
> 
> > When a user incorrectly provides an hmp command, an error response will be
> > printed that prompts the user to try "help <command name>". However, when
> > the command contains multiple parts e.g. "info uuid xyz", only the last
> > whitespace delimited string will be reported (in this example "info" will
> > be dropped and the message will read "Try "help uuid" for more information",
> > which is incorrect).
> >
> > Let's correct this by capturing the entirety of the command from the command
> > line -- excluding any extraneous characters.
> >
> > Reported-by: Mikhail Fokin <fokin@de.ibm.com>
> > Signed-off-by: Collin Walling <walling@linux.ibm.com>
> > ---
> >  monitor.c | 8 ++++++--
> >  1 file changed, 6 insertions(+), 2 deletions(-)
> >
> > diff --git a/monitor.c b/monitor.c
> > index 39f8ee1..38736b3 100644
> > --- a/monitor.c
> > +++ b/monitor.c
> > @@ -3371,6 +3371,7 @@ static void handle_hmp_command(Monitor *mon, const char *cmdline)
> >  {
> >      QDict *qdict;
> >      const mon_cmd_t *cmd;
> > +    const char *cmd_start = cmdline;
> >  
> >      trace_handle_hmp_command(mon, cmdline);
> >  
> > @@ -3381,8 +3382,11 @@ static void handle_hmp_command(Monitor *mon, const char *cmdline)
> >  
> >      qdict = monitor_parse_arguments(mon, &cmdline, cmd);
> >      if (!qdict) {
> > -        monitor_printf(mon, "Try \"help %s\" for more information\n",
> > -                       cmd->name);
> > +        while (cmdline > cmd_start && qemu_isspace(cmdline[-1])) {
> > +            cmdline--;
> > +        }
> > +        monitor_printf(mon, "Try \"help %.*s\" for more information\n",
> > +                       (int)(cmdline - cmd_start), cmd_start);
> >          return;
> >      }
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
Dr. David Alan Gilbert June 21, 2018, 11:19 a.m. UTC | #5
* Collin Walling (walling@linux.ibm.com) wrote:
> When a user incorrectly provides an hmp command, an error response will be
> printed that prompts the user to try "help <command name>". However, when
> the command contains multiple parts e.g. "info uuid xyz", only the last
> whitespace delimited string will be reported (in this example "info" will
> be dropped and the message will read "Try "help uuid" for more information",
> which is incorrect).
> 
> Let's correct this by capturing the entirety of the command from the command
> line -- excluding any extraneous characters.
> 
> Reported-by: Mikhail Fokin <fokin@de.ibm.com>
> Signed-off-by: Collin Walling <walling@linux.ibm.com>

Queued

> ---
>  monitor.c | 8 ++++++--
>  1 file changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/monitor.c b/monitor.c
> index 39f8ee1..38736b3 100644
> --- a/monitor.c
> +++ b/monitor.c
> @@ -3371,6 +3371,7 @@ static void handle_hmp_command(Monitor *mon, const char *cmdline)
>  {
>      QDict *qdict;
>      const mon_cmd_t *cmd;
> +    const char *cmd_start = cmdline;
>  
>      trace_handle_hmp_command(mon, cmdline);
>  
> @@ -3381,8 +3382,11 @@ static void handle_hmp_command(Monitor *mon, const char *cmdline)
>  
>      qdict = monitor_parse_arguments(mon, &cmdline, cmd);
>      if (!qdict) {
> -        monitor_printf(mon, "Try \"help %s\" for more information\n",
> -                       cmd->name);
> +        while (cmdline > cmd_start && qemu_isspace(cmdline[-1])) {
> +            cmdline--;
> +        }
> +        monitor_printf(mon, "Try \"help %.*s\" for more information\n",
> +                       (int)(cmdline - cmd_start), cmd_start);
>          return;
>      }
>  
> -- 
> 2.7.4
> 
> 
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
Collin Walling June 21, 2018, 3:12 p.m. UTC | #6
On 06/21/2018 07:19 AM, Dr. David Alan Gilbert wrote:
> * Collin Walling (walling@linux.ibm.com) wrote:
>> When a user incorrectly provides an hmp command, an error response will be
>> printed that prompts the user to try "help <command name>". However, when
>> the command contains multiple parts e.g. "info uuid xyz", only the last
>> whitespace delimited string will be reported (in this example "info" will
>> be dropped and the message will read "Try "help uuid" for more information",
>> which is incorrect).
>>
>> Let's correct this by capturing the entirety of the command from the command
>> line -- excluding any extraneous characters.
>>
>> Reported-by: Mikhail Fokin <fokin@de.ibm.com>
>> Signed-off-by: Collin Walling <walling@linux.ibm.com>
> 
> Queued

Thank you!

> 
>> ---
>>  monitor.c | 8 ++++++--
>>  1 file changed, 6 insertions(+), 2 deletions(-)
>>
>> diff --git a/monitor.c b/monitor.c
>> index 39f8ee1..38736b3 100644
>> --- a/monitor.c
>> +++ b/monitor.c
>> @@ -3371,6 +3371,7 @@ static void handle_hmp_command(Monitor *mon, const char *cmdline)
>>  {
>>      QDict *qdict;
>>      const mon_cmd_t *cmd;
>> +    const char *cmd_start = cmdline;
>>  
>>      trace_handle_hmp_command(mon, cmdline);
>>  
>> @@ -3381,8 +3382,11 @@ static void handle_hmp_command(Monitor *mon, const char *cmdline)
>>  
>>      qdict = monitor_parse_arguments(mon, &cmdline, cmd);
>>      if (!qdict) {
>> -        monitor_printf(mon, "Try \"help %s\" for more information\n",
>> -                       cmd->name);
>> +        while (cmdline > cmd_start && qemu_isspace(cmdline[-1])) {
>> +            cmdline--;
>> +        }
>> +        monitor_printf(mon, "Try \"help %.*s\" for more information\n",
>> +                       (int)(cmdline - cmd_start), cmd_start);
>>          return;
>>      }
>>  
>> -- 
>> 2.7.4
>>
>>
> --
> Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
>
diff mbox

Patch

diff --git a/monitor.c b/monitor.c
index 39f8ee1..38736b3 100644
--- a/monitor.c
+++ b/monitor.c
@@ -3371,6 +3371,7 @@  static void handle_hmp_command(Monitor *mon, const char *cmdline)
 {
     QDict *qdict;
     const mon_cmd_t *cmd;
+    const char *cmd_start = cmdline;
 
     trace_handle_hmp_command(mon, cmdline);
 
@@ -3381,8 +3382,11 @@  static void handle_hmp_command(Monitor *mon, const char *cmdline)
 
     qdict = monitor_parse_arguments(mon, &cmdline, cmd);
     if (!qdict) {
-        monitor_printf(mon, "Try \"help %s\" for more information\n",
-                       cmd->name);
+        while (cmdline > cmd_start && qemu_isspace(cmdline[-1])) {
+            cmdline--;
+        }
+        monitor_printf(mon, "Try \"help %.*s\" for more information\n",
+                       (int)(cmdline - cmd_start), cmd_start);
         return;
     }