Re: [Qemu-devel] [PATCH 07/17] json: learn to parse uint64 numbers

[Marc-André Lureau]

Hi

On Wed, May 31, 2017 at 2:08 PM Markus Armbruster <address@hidden> wrote:

> Marc-André Lureau <address@hidden> writes:
>
> > Hi
> >
> > On Mon, May 15, 2017 at 6:00 PM Markus Armbruster <address@hidden>
> wrote:
> >
> >> Marc-André Lureau <address@hidden> writes:
> >>
> >> > Switch strtoll() usage to qemu_strtoi64() helper while at it.
> >> >
> >> > Replace temporarily the error in qnum_get_int() with values >INT64_MAX
> >> > until the visitor is updated.
> >> >
> >> > Add a few tests for large numbers.
> >> >
> >> > Signed-off-by: Marc-André Lureau <address@hidden>
> [...]
> >> > diff --git a/qobject/qnum.c b/qobject/qnum.c
> >> > index be6307accf..2f87952db8 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) {
> >> > -            error_setg(errp, "The number is too large, use
> qnum_get_uint()");
> >> > -            return 0;
> >> > +            /* temporarily accepts to cast to i64 until visitor is
> switched */
> >> > +            error_report("The number is too large, use
> qnum_get_uint()");
> >>
> >> Awkward.  Can we avoid this somehow?
> >>
> >>
> > by merging the next two patches? I think it's easier to understand why
> and
> > how I came up with the following fix.
>
> I played with it a bit.  Here's something that appears to do the trick:
>
> PATCH 07: Drop changes to qobject/qnum.c tests/check-qnum.c
>           Move change to tests/test-qobject-input-visitor.c to PATCH 08
> PATCH 08: Move PATCH 09's change to hw/i386/acpi-build.c here
> PATCH 09: No need to revert the changes dropped in PATCH 07; patch is
>           now empty
>
> Less churn, no temporary breakage, same end result.
>

ok


>
> Pushed as branch hack-qapi-qnum for your convenience:
> https://repo.or.cz/qemu/armbru.git/shortlog/refs/heads/hack-qapi-qnum
>
> Original is on branch review-qapi-qnum.
>
>
thanks that helped

> >> >          }
> >> >          return qn->u.u64;
> >> >      case QNUM_DOUBLE:
> [...]
>
-- 
Marc-André Lureau