Re: [Qemu-devel] [PATCH 02/14] qlit: move qlit from check-qjson to qobject/

[Eduardo Habkost]

On Fri, Sep 01, 2017 at 11:05:11AM +0200, Markus Armbruster wrote:
> Eduardo Habkost <address@hidden> writes:
> 
> > On Thu, Aug 24, 2017 at 12:33:38PM +0200, Marc-André Lureau wrote:
> >> Fix code style issues while at it, to please check-patch.
> >> 
> >> Signed-off-by: Marc-André Lureau <address@hidden>
> >> ---
> >>  include/qapi/qmp/qlit.h | 49 +++++++++++++++++++++++++
> >>  qobject/qlit.c          | 89 +++++++++++++++++++++++++++++++++++++++++++++
> >>  tests/check-qjson.c     | 96 
> >> +------------------------------------------------
> >>  qobject/Makefile.objs   |  2 +-
> >>  4 files changed, 140 insertions(+), 96 deletions(-)
> >>  create mode 100644 include/qapi/qmp/qlit.h
> >>  create mode 100644 qobject/qlit.c
> >> 
> >> diff --git a/include/qapi/qmp/qlit.h b/include/qapi/qmp/qlit.h
> >> new file mode 100644
> >> index 0000000000..4e2e760ef1
> >> --- /dev/null
> >> +++ b/include/qapi/qmp/qlit.h
> >> @@ -0,0 +1,49 @@
> >> +/*
> >> + * Copyright IBM, Corp. 2009
> >> + * Copyright (c) 2013, 2015, 2017 Red Hat Inc.
> >> + *
> >> + * Authors:
> >> + *  Anthony Liguori   <address@hidden>
> >> + *  Markus Armbruster <address@hidden>
> >> + *  Marc-André Lureau <address@hidden>
> >> + *
> >> + * This work is licensed under the terms of the GNU LGPL, version 2.1 or 
> >> later.
> >> + * See the COPYING.LIB file in the top-level directory.
> >> + *
> >> + */
> >> +#ifndef QLIT_H_
> >> +#define QLIT_H_
> >> +
> >> +#include "qapi-types.h"
> >> +#include "qobject.h"
> >> +
> >> +typedef struct LiteralQDictEntry LiteralQDictEntry;
> >> +typedef struct LiteralQObject LiteralQObject;
> >> +
> >> +struct LiteralQObject {
> >> +    int type;
> >> +    union {
> >> +        int64_t qnum;
> >> +        const char *qstr;
> >> +        LiteralQDictEntry *qdict;
> >> +        LiteralQObject *qlist;
> >> +    } value;
> >> +};
> >> +
> >> +struct LiteralQDictEntry {
> >> +    const char *key;
> >> +    LiteralQObject value;
> >> +};
> >> +
> >> +#define QLIT_QNUM(val) \
> >> +    (LiteralQObject){.type = QTYPE_QNUM, .value.qnum = (val)}
> >> +#define QLIT_QSTR(val) \
> >> +    (LiteralQObject){.type = QTYPE_QSTRING, .value.qstr = (val)}
> >> +#define QLIT_QDICT(val) \
> >> +    (LiteralQObject){.type = QTYPE_QDICT, .value.qdict = (val)}
> >> +#define QLIT_QLIST(val) \
> >> +    (LiteralQObject){.type = QTYPE_QLIST, .value.qlist = (val)}
> >
> > I'm still trying to understand why this exists.  Doesn't this
> > provide exactly the same functionality as QObject?
> 
> The value-add is initializers.  Check out check-qjson.c for examples.

Oh, I see.

QList and QDict seem to be the ones that require special code for
literals.  Making initializers for QNull, QNum, QString, and
QBool seems possible.

> 
> Use cases:
> 
> * Specify expected output as data (QLitObject initializer), then use
>   qlit_equal_qobject() to verify actual output matches.  Example:
>   check-qjson.c.  I think it compares nicely to the qdict_get morass we
>   use elsewhere.

Is any qlit_equal_qobject() user so performance-critical that it
couldn't be replaced by a QObject/QObject comparison function,
and implemented as qobject_equals(qobject_from_qlit(a), b)?


> 
> * Specify a QObject as data (QLitObject initializer), build it with
>   qobject_from_qlit().  Example: PATCH 14.  I think it beats specifying
>   the QObject in code (qdict_new(), qdict_put(), ...) at least when the
>   QObject is big.

-- 
Eduardo