Re: [Qemu-devel] [PATCH v3 06/43] qapi: merge QInt and QFloat in QNum

[Markus Armbruster]

Marc-André Lureau <address@hidden> writes:

> We would like to use a same QObject type to represent numbers, whether
> they are int, uint, or floats. Getters will allow some compatibility
> between the various types if the number fits other representations.
>
> Add a few more tests while at it.
>
> Signed-off-by: Marc-André Lureau <address@hidden>
[...]
> diff --git a/blockdev.c b/blockdev.c
> index 892d768574..154c95d402 100644
> --- a/blockdev.c
> +++ b/blockdev.c
> @@ -334,9 +334,11 @@ static bool parse_stats_intervals(BlockAcctStats *stats, 
> QList *intervals,
>              break;
>          }
>  
> -        case QTYPE_QINT: {
> -            int64_t length = qint_get_int(qobject_to_qint(entry->value));
> -            if (length > 0 && length <= UINT_MAX) {
> +        case QTYPE_QNUM: {
> +            int64_t length;
> +
> +            if (qnum_get_try_int(qobject_to_qnum(entry->value), &length) &&
> +                length > 0 && length <= UINT_MAX) {
>                  block_acct_add_interval(stats, (unsigned) length);
>              } else {
>                  error_setg(errp, "Invalid interval length: %" PRId64, 
> length);

Less churn and a bit more legible:

  +        case QTYPE_QNUM: {
  +            int64_t length = qnum_get_int(qobject_to_qint(entry->value));
  +
  +            if (length > 0 && length <= UINT_MAX) {

Aborts when entry->value doesn't fit into int64_t, but that's no worse
than before.

[...]
> diff --git a/tests/test-qobject-input-visitor.c 
> b/tests/test-qobject-input-visitor.c
> index 3a6ce2226c..9e3d4aa7e5 100644
> --- a/tests/test-qobject-input-visitor.c
> +++ b/tests/test-qobject-input-visitor.c
> @@ -164,9 +164,9 @@ static void 
> test_visitor_in_int_overflow(TestInputVisitorData *data,
>      Error *err = NULL;
>      Visitor *v;
>  
> -    /* this will overflow a Qint/int64, so should be deserialized into
> -     * a QFloat/double field instead, leading to an error if we pass it
> -     * to visit_type_int. confirm this.
> +    /* this will overflow a QNUM_I64, so should be deserialized into a

Wing both ends and start with a capital letter, please:

  +    /*
  +     * This will overflow a QNUM_I64, so should be deserialized into a

> +     * QNUM_DOUBLE field instead, leading to an error if we pass it to
> +     * visit_type_int. confirm this.

Humor me:

        * visit_type_int().  Confirm this.

>       */
>      v = visitor_input_test_init(data, "%f", DBL_MAX);
>  
[...]

With these minor touch-ups:
Reviewed-by: Markus Armbruster <address@hidden>