On 05/04/2018 02:19 PM, Eric Blake wrote:
> On 05/04/2018 01:02 PM, Collin Walling wrote:
>
>>> So rather than trying to reconstruct a string, you could reuse what you
>>> already have. This is a shorter patch that I think accomplishes the same
>>> goal:
>>>
>>> diff --git i/monitor.c w/monitor.c
>>> index 39f8ee17ba7..38736b3a20d 100644
>>> --- i/monitor.c
>>> +++ w/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;
>>> }
>>>
>>>
>>>
>>
>> Very interesting... you managed to reuse what was in cmdline without
>> printing anything extraneous that
>> the user might have provided... nicely done!
>>
>> Your print statement is intriguing to me... I'm not entirely sure how it
>> works.
>
> The format specifiers in printf are %[flags][width][.precision]format. So I'm
> requesting a precision-limited string print (which says the maximum number of
> characters to print, rather than the usual semantics of printing until the
> trailing NUL is found), and the precision of .* (instead of a more typical .1
> or similar) says that the precision will be an int argument to printf rather
> than inline (the width argument can also be passed via *). The cast to int
> is annoyingly part of the specification (subtracting two pointers within a
> string results in a ptrdiff_t, but on 64-bit platforms, ptrdiff_t and int are
> not equally handled through vararg functions). And that exact style of
> printf() magic was already in use in monitor_parse_command() that your patch
> attempt was touching (see where cmdp_start is used). And if you really want
> weird, 'man 3 printf' states that "%.*s" is equivalent to "%2$.*1$s" (the $
> syntax is for cases cases where you need to consume printf arguments
> out-of-order, often when dealing with translated strings).
>
Very cool! Thank you for clearing that up for me. I must've glanced over the
usage in monitor_parse_command().
>>
>> How would you like to move forward with this patch?
>
> I'm more than happy to let you post a v2 of the patch, incorporating the
> ideas you just learned from me. (Or, if you do nothing, then in a week or
> so, I'll probably notice the patch is still sitting unapplied in my local
> repository and submit it myself - but then I wouldn't be helping the qemu
> community grow...)
>
Much appreciated. I'll post v2 as a reply to this email chain (since it's very
small and I don't expect much discussion to follow)
with your suggested changes.
--
Respectfully,
- Collin Walling
