| Message ID | 20170824103350.16400-3-marcandre.lureau@redhat.com |
|---|---|
| State | New |
| Headers | show |
Marc-André Lureau <marcandre.lureau@redhat.com> writes: > Fix code style issues while at it, to please check-patch. It's spelled checkpatch. Can touch up on commit. > Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com> > --- > 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 <aliguori@us.ibm.com> > + * Markus Armbruster <armbru@redhat.com> > + * Marc-André Lureau <marcandre.lureau@redhat.com> > + * > + * 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_ QLIT_H (no trailing _), to match the other headers in this directory. Can touch up on commit. [...] With the two spelling nits corrected: Reviewed-by: Markus Armbruster <armbru@redhat.com>
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 <marcandre.lureau@redhat.com> > --- > 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 <aliguori@us.ibm.com> > + * Markus Armbruster <armbru@redhat.com> > + * Marc-André Lureau <marcandre.lureau@redhat.com> > + * > + * 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? > [...]
Eduardo Habkost <ehabkost@redhat.com> 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 <marcandre.lureau@redhat.com> >> --- >> 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 <aliguori@us.ibm.com> >> + * Markus Armbruster <armbru@redhat.com> >> + * Marc-André Lureau <marcandre.lureau@redhat.com> >> + * >> + * 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. 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. * 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.
On Fri, Sep 01, 2017 at 11:05:11AM +0200, Markus Armbruster wrote: > Eduardo Habkost <ehabkost@redhat.com> 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 <marcandre.lureau@redhat.com> > >> --- > >> 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 <aliguori@us.ibm.com> > >> + * Markus Armbruster <armbru@redhat.com> > >> + * Marc-André Lureau <marcandre.lureau@redhat.com> > >> + * > >> + * 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.
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 <aliguori@us.ibm.com> + * Markus Armbruster <armbru@redhat.com> + * Marc-André Lureau <marcandre.lureau@redhat.com> + * + * 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)} + +int compare_litqobj_to_qobj(LiteralQObject *lhs, QObject *rhs); + +#endif /* QLIT_H_ */ diff --git a/qobject/qlit.c b/qobject/qlit.c new file mode 100644 index 0000000000..5917c3584e --- /dev/null +++ b/qobject/qlit.c @@ -0,0 +1,89 @@ +/* + * QLit literal qobject + * + * Copyright IBM, Corp. 2009 + * Copyright (c) 2013, 2015, 2017 Red Hat Inc. + * + * Authors: + * Anthony Liguori <aliguori@us.ibm.com> + * Markus Armbruster <armbru@redhat.com> + * Marc-André Lureau <marcandre.lureau@redhat.com> + * + * 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. + */ + +#include "qemu/osdep.h" + +#include "qapi/qmp/qlit.h" +#include "qapi/qmp/types.h" + +typedef struct QListCompareHelper { + int index; + LiteralQObject *objs; + int result; +} QListCompareHelper; + +static void compare_helper(QObject *obj, void *opaque) +{ + QListCompareHelper *helper = opaque; + + if (helper->result == 0) { + return; + } + + if (helper->objs[helper->index].type == QTYPE_NONE) { + helper->result = 0; + return; + } + + helper->result = + compare_litqobj_to_qobj(&helper->objs[helper->index++], obj); +} + +int compare_litqobj_to_qobj(LiteralQObject *lhs, QObject *rhs) +{ + int64_t val; + + if (!rhs || lhs->type != qobject_type(rhs)) { + return 0; + } + + switch (lhs->type) { + case QTYPE_QNUM: + g_assert(qnum_get_try_int(qobject_to_qnum(rhs), &val)); + return lhs->value.qnum == val; + case QTYPE_QSTRING: + return (strcmp(lhs->value.qstr, + qstring_get_str(qobject_to_qstring(rhs))) == 0); + case QTYPE_QDICT: { + int i; + + for (i = 0; lhs->value.qdict[i].key; i++) { + QObject *obj = qdict_get(qobject_to_qdict(rhs), + lhs->value.qdict[i].key); + + if (!compare_litqobj_to_qobj(&lhs->value.qdict[i].value, obj)) { + return 0; + } + } + + return 1; + } + case QTYPE_QLIST: { + QListCompareHelper helper; + + helper.index = 0; + helper.objs = lhs->value.qlist; + helper.result = 1; + + qlist_iter(qobject_to_qlist(rhs), compare_helper, &helper); + + return helper.result; + } + default: + break; + } + + return 0; +} diff --git a/tests/check-qjson.c b/tests/check-qjson.c index a3a97b0d99..525f79e60b 100644 --- a/tests/check-qjson.c +++ b/tests/check-qjson.c @@ -16,6 +16,7 @@ #include "qapi/error.h" #include "qapi/qmp/types.h" #include "qapi/qmp/qjson.h" +#include "qapi/qmp/qlit.h" #include "qemu-common.h" static void escaped_string(void) @@ -1059,101 +1060,6 @@ static void keyword_literal(void) QDECREF(null); } -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)} - -typedef struct QListCompareHelper -{ - int index; - LiteralQObject *objs; - int result; -} QListCompareHelper; - -static int compare_litqobj_to_qobj(LiteralQObject *lhs, QObject *rhs); - -static void compare_helper(QObject *obj, void *opaque) -{ - QListCompareHelper *helper = opaque; - - if (helper->result == 0) { - return; - } - - if (helper->objs[helper->index].type == QTYPE_NONE) { - helper->result = 0; - return; - } - - helper->result = compare_litqobj_to_qobj(&helper->objs[helper->index++], obj); -} - -static int compare_litqobj_to_qobj(LiteralQObject *lhs, QObject *rhs) -{ - int64_t val; - - if (!rhs || lhs->type != qobject_type(rhs)) { - return 0; - } - - switch (lhs->type) { - case QTYPE_QNUM: - g_assert(qnum_get_try_int(qobject_to_qnum(rhs), &val)); - return lhs->value.qnum == val; - case QTYPE_QSTRING: - return (strcmp(lhs->value.qstr, qstring_get_str(qobject_to_qstring(rhs))) == 0); - case QTYPE_QDICT: { - int i; - - for (i = 0; lhs->value.qdict[i].key; i++) { - QObject *obj = qdict_get(qobject_to_qdict(rhs), lhs->value.qdict[i].key); - - if (!compare_litqobj_to_qobj(&lhs->value.qdict[i].value, obj)) { - return 0; - } - } - - return 1; - } - case QTYPE_QLIST: { - QListCompareHelper helper; - - helper.index = 0; - helper.objs = lhs->value.qlist; - helper.result = 1; - - qlist_iter(qobject_to_qlist(rhs), compare_helper, &helper); - - return helper.result; - } - default: - break; - } - - return 0; -} - static void simple_dict(void) { int i; diff --git a/qobject/Makefile.objs b/qobject/Makefile.objs index fc8885c9a4..002d25873a 100644 --- a/qobject/Makefile.objs +++ b/qobject/Makefile.objs @@ -1,2 +1,2 @@ -util-obj-y = qnull.o qnum.o qstring.o qdict.o qlist.o qbool.o +util-obj-y = qnull.o qnum.o qstring.o qdict.o qlist.o qbool.o qlit.o util-obj-y += qjson.o qobject.o json-lexer.o json-streamer.o json-parser.o
Fix code style issues while at it, to please check-patch. Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com> --- 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