Re: [Qemu-devel] [PATCH v3 2/7] qapi: correctly parse uint64_t values from strings

[David Gibson]

On Tue, Oct 23, 2018 at 05:23:01PM +0200, David Hildenbrand wrote:
> Right now, we parse uint64_t values just like int64_t values, resulting
> in negative values getting accepted and certain valid large numbers only
> being representable as negative numbers. Also, reported errors indicate
> that an int64_t is expected.
> 
> Parse uin64_t separately. We don't have to worry about ranges.
> 
> E.g. we can now also specify
>     -device nvdimm,memdev=mem1,id=nv1,addr=0xFFFFFFFFC0000000
> Instead of only going via negative values
>     -device nvdimm,memdev=mem1,id=nv1,addr=-0x40000000
> 
> Resulting in the same values
> 
> (qemu) info memory-devices
> Memory device [nvdimm]: "nv1"
>   addr: 0xffffffffc0000000
>   slot: 0
>   node: 0
> 
> Signed-off-by: David Hildenbrand <address@hidden>
> ---
>  qapi/string-input-visitor.c | 17 +++++++++--------
>  1 file changed, 9 insertions(+), 8 deletions(-)
> 
> diff --git a/qapi/string-input-visitor.c b/qapi/string-input-visitor.c
> index c1454f999f..f2df027325 100644
> --- a/qapi/string-input-visitor.c
> +++ b/qapi/string-input-visitor.c
> @@ -247,15 +247,16 @@ error:
>  static void parse_type_uint64(Visitor *v, const char *name, uint64_t *obj,
>                                Error **errp)
>  {
> -    /* FIXME: parse_type_int64 mishandles values over INT64_MAX */
> -    int64_t i;
> -    Error *err = NULL;
> -    parse_type_int64(v, name, &i, &err);
> -    if (err) {
> -        error_propagate(errp, err);
> -    } else {
> -        *obj = i;
> +    StringInputVisitor *siv = to_siv(v);
> +    uint64_t val;
> +
> +    if (qemu_strtou64(siv->string, NULL, 0, &val)) {
> +        error_setg(errp, QERR_INVALID_PARAMETER_VALUE, name ? name : "null",
> +                   "an uint64 value");
> +        return;
>      }
> +
> +    *obj = val;
>  }

It's not obvious to me why this looks so different from the code in
parse_type_int64().  Should we be using qemu_strtoi64() in the
pre-existing function, instead of what's there now?

>  
>  static void parse_type_size(Visitor *v, const char *name, uint64_t *obj,

-- 
David Gibson                    | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au  | minimalist, thank you.  NOT _the_ _other_
                                | _way_ _around_!
http://www.ozlabs.org/~dgibson

signature.asc
Description: PGP signature