Re: [Qemu-devel] [PATCH v2 14/45] qapi: update the qobject visitor to use QNUM_U64

[Marc-André Lureau]

----- Original Message -----
> One more nitpick:
> 
> Marc-André Lureau <address@hidden> writes:
> 
> > Switch to use QNum/uint where appropriate to remove i64 limitation.
> >
> > The input visitor will cast i64 input to u64 for compatibility
> > reasons (existing json QMP client already use negative i64 for large
> > u64, and expect an implicit cast in qemu).
> >
> > Signed-off-by: Marc-André Lureau <address@hidden>
> [...]
> > diff --git a/tests/test-qobject-output-visitor.c
> > b/tests/test-qobject-output-visitor.c
> > index 3180d8cbde..d9f106d52e 100644
> > --- a/tests/test-qobject-output-visitor.c
> > +++ b/tests/test-qobject-output-visitor.c
> > @@ -602,17 +602,31 @@ static void check_native_list(QObject *qobj,
> >      qlist = qlist_copy(qobject_to_qlist(qdict_get(qdict, "data")));
> >  
> >      switch (kind) {
> > -    case USER_DEF_NATIVE_LIST_UNION_KIND_S8:
> > -    case USER_DEF_NATIVE_LIST_UNION_KIND_S16:
> > -    case USER_DEF_NATIVE_LIST_UNION_KIND_S32:
> > -    case USER_DEF_NATIVE_LIST_UNION_KIND_S64:
> >      case USER_DEF_NATIVE_LIST_UNION_KIND_U8:
> >      case USER_DEF_NATIVE_LIST_UNION_KIND_U16:
> >      case USER_DEF_NATIVE_LIST_UNION_KIND_U32:
> >      case USER_DEF_NATIVE_LIST_UNION_KIND_U64:
> > -        /* all integer elements in JSON arrays get stored into QNums when
> > -         * we convert to QObjects, so we can check them all in the same
> > -         * fashion, so simply fall through here
> > +        for (i = 0; i < 32; i++) {
> > +            QObject *tmp;
> > +            QNum *qvalue;
> > +            uint64_t val;
> > +
> > +            tmp = qlist_peek(qlist);
> > +            g_assert(tmp);
> > +            qvalue = qobject_to_qnum(tmp);
> > +            g_assert(qnum_get_uint(qvalue, &val));
> > +            g_assert_cmpuint(val, ==, i);
> > +            qobject_decref(qlist_pop(qlist));
> > +        }
> > +        break;
> > +
> > +    case USER_DEF_NATIVE_LIST_UNION_KIND_S8:
> > +    case USER_DEF_NATIVE_LIST_UNION_KIND_S16:
> > +    case USER_DEF_NATIVE_LIST_UNION_KIND_S32:
> > +    case USER_DEF_NATIVE_LIST_UNION_KIND_S64:
> > +        /* All signed integer elements in JSON arrays get stored into
> > +         * QInts when we convert to QObjects, so we can check them all
> > +         * in the same fashion, so simply fall through here.
> >           */
> >      case USER_DEF_NATIVE_LIST_UNION_KIND_INTEGER:
> >          for (i = 0; i < 32; i++) {
> 
> Wing both ends of the comment, please.

You want this: ?

/* All signed integer elements in JSON arrays get stored into */
/* QInts when we convert to QObjects, so we can check them all */
...

I think this is quite annoying for refilling with emacs

Perhaps rather

/* All signed integer elements in JSON arrays get stored into
 * QInts when we convert to QObjects, so we can check them all */

quite uglier to me, but I don't care

However, I'd prefer if either we have a common rule in qemu or we don't 
bike-sched over that...

(even better would be to have this somehow automated with tools like 
git-clang-format)