Re: [Qemu-devel] [PATCH] monitor: HMP: fix consecutive integer expression parsing

[Markus Armbruster]

Blue Swirl <address@hidden> writes:

> On Fri, Aug 5, 2011 at 9:08 PM, Anthony Liguori <address@hidden> wrote:
>> On 08/05/2011 03:39 PM, Blue Swirl wrote:
>>>
>>> On Fri, Aug 5, 2011 at 4:51 PM, Anthony Liguori<address@hidden>
>>>  wrote:
>>>>
>>>> On 08/03/2011 06:57 AM, Alon Levy wrote:
>>>>>
>>>>> Currently a command that takes two consecutive integer operations, like
>>>>> client_migrate_info, will be incorrectly parsed by the human monitor if
>>>>> the second expression begins with a minus ('-') or plus ('+') sign:
>>>>>
>>>>> client_migrate_info<protocol>    <hostname>    <port>    <tls-port>
>>>>> client_migrate_info spice localhost 5900 -1
>>>>> =>    port = 5899 = 5900 - 1
>>>>>    tls-port = -1
>>>>> But expected by the user to be:
>>>>>    port = 5900
>>>>>    tls-port = -1
>>>>>
>>>>> The fix is that for any required integer (ilM) expression followed by
>>>>> another
>>>>> integer expression (ilM) the first expression will be parsed by
>>>>> expr_unary
>>>>> instead of expr_sum. So you can still use arithmetic, but you have to
>>>>> enclose
>>>>> it in parenthesis:
>>>>>
>>>>> Command line | Old parsed result | With patch result
>>>>> (1+1) 2      | 2, 2              | 2, 2
>>>>> 1 -1         | 0, -1             | 1, -1
>>>>> The rest are bizarre but not any worse then before
>>>>> 1+2+3        | 6, 5              | 1, 5
>>>>> (1+2)+3      | 3, 3              | 3, 3
>>>>
>>>> I vote for just removing the expression parsing entirely.  It's
>>>> incredibly
>>>> non-intuitive and I don't think anyone really uses it.
>>>>
>>>> Does anyone strongly object?
>>>
>>> I think the expressions would be useful with memory addresses, like
>>> "xp/i $pc-4", but I usually start GDB in these cases. Can we disable
>>> the expressions only for ports?
>>
>> Not sure what you mean by ports.  You mean for anything but vc?  My goal in
>> disabling the expressions would be to simplify the parsing by removing all
>> that messy code.
>
> Retain the parsing for only memory addresses, remove from other areas.

Feasible, but we'd still be open to ambiguities around addresses, and
we'd still be maintaining all that messy code.

> Another way would be to require any expressions to be enclosed in
> parentheses for all cases.

Reduces the ambiguities, but some remain.

Is (1 + 2) one argument (which can evaluate into the integer 3), or
three arguments (which can evaluate into the strings/filenames/whatever
"(1", "+" and "2)")?  Depends on argument types, just like it does
without parenthesis.

> But I don't object to removing the code very much, as I said I use
> GDB. Also the setjmp stuff is buggy.

We have more important problems to solve than providing our users with
yet another pocket calculator.