Re: [Qemu-devel] [PATCH 2/2] hmp: fix bad value conversion for M type

[Markus Armbruster]

Luiz Capitulino <address@hidden> writes:

> The M type converts from megabytes to bytes. However, the value can be
> negative before the conversion, which will lead to a flawed conversion.
>
> For example, this:
>
>  (qemu) balloon -1000000000000011
>  (qemu)
>
> Just "works", but the value passed by the balloon command will be
> something else.
>
> This patch fixes this problem by requering a positive value before
> converting. There's really no reason to accept a negative value for
> the M type.
>
> Signed-off-by: Luiz Capitulino <address@hidden>
> Reviewed-by: Eric Blake <address@hidden>
> ---
>  monitor.c |    4 ++++
>  1 file changed, 4 insertions(+)
>
> diff --git a/monitor.c b/monitor.c
> index 56ee971..58b692b 100644
> --- a/monitor.c
> +++ b/monitor.c
> @@ -3625,6 +3625,10 @@ static const mon_cmd_t *monitor_parse_command(Monitor 
> *mon,
>                      monitor_printf(mon, "integer is for 32-bit values\n");
>                      goto fail;
>                  } else if (c == 'M') {
> +                    if (val < 0) {
> +                        monitor_printf(mon, "enter a positive value\n");
> +                        goto fail;
> +                    }
>                      val <<= 20;
>                  }
>                  qdict_put(qdict, key, qint_from_int(val));

Error message doesn't tell the user which argument is wrong.  All the
existing messages are just as bad.  Let's stick to fixing the bug at
hand, and leave decent error messages to another day.

Please fix up the comment on type 'M':

 * 'M'          just like 'l', except in user mode the value is
 *              multiplied by 2^20 (think Mebibyte)

It's no longer "just like 'l'", since unlike 'l' it doesn't accept
negative numbers.