Re: [Qemu-devel] [PATCH 09/17] qnum: fix get_int() with values > INT64_MAX

[Markus Armbruster]

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

> Now that the visitor has been switch to use qnum_uint, fix the bad
> get_int() to use get_uint() instead. Remove compatibility code.
>
> Signed-off-by: Marc-André Lureau <address@hidden>
> ---
>  hw/i386/acpi-build.c | 2 +-
>  qobject/qnum.c       | 4 ++--
>  tests/check-qnum.c   | 9 ++++-----
>  3 files changed, 7 insertions(+), 8 deletions(-)
>
> diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
> index ec3ae7fa85..767da5d78e 100644
> --- a/hw/i386/acpi-build.c
> +++ b/hw/i386/acpi-build.c
> @@ -2585,7 +2585,7 @@ static bool acpi_get_mcfg(AcpiMcfgInfo *mcfg)
>      if (!o) {
>          return false;
>      }
> -    mcfg->mcfg_base = qnum_get_int(qobject_to_qnum(o), &error_abort);
> +    mcfg->mcfg_base = qnum_get_uint(qobject_to_qnum(o), &error_abort);
>      qobject_decref(o);
>  
>      o = object_property_get_qobject(pci_host, PCIE_HOST_MCFG_SIZE, NULL);

The change makes sense becaise mcfg_base is uint64_t.  But why does it
belong to this patch?

> diff --git a/qobject/qnum.c b/qobject/qnum.c
> index 2f87952db8..be6307accf 100644
> --- a/qobject/qnum.c
> +++ b/qobject/qnum.c
> @@ -76,8 +76,8 @@ int64_t qnum_get_int(const QNum *qn, Error **errp)
>          return qn->u.i64;
>      case QNUM_U64:
>          if (qn->u.u64 > INT64_MAX) {
> -            /* temporarily accepts to cast to i64 until visitor is switched 
> */
> -            error_report("The number is too large, use qnum_get_uint()");
> +            error_setg(errp, "The number is too large, use qnum_get_uint()");
> +            return 0;
>          }
>          return qn->u.u64;
>      case QNUM_DOUBLE:
> diff --git a/tests/check-qnum.c b/tests/check-qnum.c
> index 9a22af3d0e..8199546f99 100644
> --- a/tests/check-qnum.c
> +++ b/tests/check-qnum.c
> @@ -107,11 +107,10 @@ static void qnum_get_uint_test(void)
>      error_free_or_abort(&err);
>      QDECREF(qn);
>  
> -    /* temporarily disabled until visitor is switched */
> -    /* qn = qnum_from_uint(-1ULL); */
> -    /* qnum_get_int(qn, &err); */
> -    /* error_free_or_abort(&err); */
> -    /* QDECREF(qn); */
> +    qn = qnum_from_uint(-1ULL);
> +    qnum_get_int(qn, &err);
> +    error_free_or_abort(&err);
> +    QDECREF(qn);
>  
>      /* invalid case */
>      qn = qnum_from_double(0.42);