On Wed, Aug 03, 2011 at 02:57:27PM +0300, 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
Actually since the current code doesn't accept negative numbers after this patch
that command is illegal, which is perfectly fine.
>
> 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
The old is actually: 6, -1 (the first get_expr consumes the whole command line,
the
second gets a zero length string and so returns the default -1)
> (1+2)+3 | 3, 3 | 3, 3
>
> Signed-off-by: Alon Levy <al...@redhat.com>
> ---
> monitor.c | 27 ++++++++++++++++++++++++---
> 1 files changed, 24 insertions(+), 3 deletions(-)
>
> diff --git a/monitor.c b/monitor.c
> index 1b8ba2c..45e2d6c 100644
> --- a/monitor.c
> +++ b/monitor.c
> @@ -3889,7 +3889,7 @@ static int64_t expr_sum(Monitor *mon)
> return val;
> }
>
> -static int get_expr(Monitor *mon, int64_t *pval, const char **pp)
> +static int get_expr(Monitor *mon, int64_t *pval, const char **pp, int unary)
> {
> pch = *pp;
> if (setjmp(expr_env)) {
> @@ -3898,7 +3898,11 @@ static int get_expr(Monitor *mon, int64_t *pval, const
> char **pp)
> }
> while (qemu_isspace(*pch))
> pch++;
> - *pval = expr_sum(mon);
> + if (unary) {
> + *pval = expr_unary(mon);
> + } else {
> + *pval = expr_sum(mon);
> + }
> *pp = pch;
> return 0;
> }
> @@ -4267,6 +4271,9 @@ static const mon_cmd_t *monitor_parse_command(Monitor
> *mon,
> case 'M':
> {
> int64_t val;
> + int unary = 0;
> + char *next_key;
> + char *next;
>
> while (qemu_isspace(*p))
> p++;
> @@ -4288,7 +4295,21 @@ static const mon_cmd_t *monitor_parse_command(Monitor
> *mon,
> }
> typestr++;
> }
> - if (get_expr(mon, &val, &p))
> + next = key_get_info(typestr, &next_key);
> + qemu_free(next_key);
> + if (*next == 'i' || *next == 'l' || *next == 'M') {
> + /* If a command has two consecutive ii parameters the
> first
> + * get_expr will also parse the second parameter if it
> + * starts with a - or +. To avoid this only parse unary
> in
> + * this case, i.e.:
> + * client_migrate_info spice localhost 1 -1
> + * => 1, -1
> + * client_migrate_info spice localhost (1+3) -1
> + * => 4, -1
> + */
> + unary = 1;
> + }
> + if (get_expr(mon, &val, &p, unary))
> goto fail;
> /* Check if 'i' is greater than 32-bit */
> if ((c == 'i') && ((val >> 32) & 0xffffffff)) {
> --
> 1.7.6
>
>
