[Qemu-devel] [PATCH v2 00/14] Generate a literal qobject for introspection

[Qemu-devel] [PATCH v2 00/14] Generate a literal qobject for introspection

[Marc-André Lureau]

Hi,

This series is based on patches 2-5 from "[PATCH v2 00/54] qapi: add #if
pre-processor conditions to generated code". Generating a literal qobject
will allow to easily introduce #if conditionals from the original series.

v2:
- use QLIST_FOREACH_ENTRY for list comparison
- update qdict comparison to check for the size instead of doing a
  qdict copy
- commit message update
- add r-b tags

Marc-André Lureau (14):
  qdict: add qdict_put_null() helper
  qlit: move qlit from check-qjson to qobject/
  qlit: use QLit prefix consistently
  qlit: remove compound literals
  qlit: rename compare_litqobj_to_qobj() to qlit_equal_qobject()
  qlit: make qlit_equal_qobject return a bool
  qlit: make qlit_equal_qobject() take const arguments
  qlit: add QLIT_QNULL and QLIT_BOOL
  qlit: Replace open-coded qnum_get_int() by call
  tests: add qlit tests
  qlit: improve QLit dict vs qdict comparison
  qlit: improve QLit list vs qlist comparison
  qlit: add qobject_form_qlit()
  qapi: generate a literal qobject for introspection

 scripts/qapi-introspect.py         |  87 ++++++++++++---------
 include/qapi/qmp/qdict.h           |   4 +-
 include/qapi/qmp/qlit.h            |  56 ++++++++++++++
 monitor.c                          |   2 +-
 qobject/qlit.c                     | 121 ++++++++++++++++++++++++++++++
 target/i386/cpu.c                  |   4 +-
 tests/check-qjson.c                | 150 +++++++------------------------------
 tests/check-qlit.c                 | 100 +++++++++++++++++++++++++
 tests/test-qobject-input-visitor.c |  11 ++-
 docs/devel/qapi-code-gen.txt       |  29 ++++---
 qobject/Makefile.objs              |   2 +-
 tests/Makefile.include             |   5 +-
 12 files changed, 393 insertions(+), 178 deletions(-)
 create mode 100644 include/qapi/qmp/qlit.h
 create mode 100644 qobject/qlit.c
 create mode 100644 tests/check-qlit.c

-- 
2.14.1.146.gd35faa819

[Qemu-devel] [PATCH v2 01/14] qdict: add qdict_put_null() helper

[Marc-André Lureau]

A step towards completeness.

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
---
 include/qapi/qmp/qdict.h | 4 +++-
 target/i386/cpu.c        | 4 ++--
 2 files changed, 5 insertions(+), 3 deletions(-)

diff --git a/include/qapi/qmp/qdict.h b/include/qapi/qmp/qdict.h
index 363e431106..6588c7f0c8 100644
--- a/include/qapi/qmp/qdict.h
+++ b/include/qapi/qmp/qdict.h
@@ -53,13 +53,15 @@ void qdict_destroy_obj(QObject *obj);
 #define qdict_put(qdict, key, obj) \
         qdict_put_obj(qdict, key, QOBJECT(obj))
 
-/* Helpers for int, bool, and string */
+/* Helpers for int, bool, null, and string */
 #define qdict_put_int(qdict, key, value) \
         qdict_put(qdict, key, qnum_from_int(value))
 #define qdict_put_bool(qdict, key, value) \
         qdict_put(qdict, key, qbool_from_bool(value))
 #define qdict_put_str(qdict, key, value) \
         qdict_put(qdict, key, qstring_from_str(value))
+#define qdict_put_null(qdict, key) \
+        qdict_put(qdict, key, qnull())
 
 /* High level helpers */
 double qdict_get_double(const QDict *qdict, const char *key);
diff --git a/target/i386/cpu.c b/target/i386/cpu.c
index ddc45abd70..bec2009a9c 100644
--- a/target/i386/cpu.c
+++ b/target/i386/cpu.c
@@ -2454,7 +2454,7 @@ static QDict *x86_cpu_static_props(void)
 
     d = qdict_new();
     for (i = 0; props[i]; i++) {
-        qdict_put(d, props[i], qnull());
+        qdict_put_null(d, props[i]);
     }
 
     for (w = 0; w < FEATURE_WORDS; w++) {
@@ -2464,7 +2464,7 @@ static QDict *x86_cpu_static_props(void)
             if (!fi->feat_names[bit]) {
                 continue;
             }
-            qdict_put(d, fi->feat_names[bit], qnull());
+            qdict_put_null(d, fi->feat_names[bit]);
         }
     }
 
-- 
2.14.1.146.gd35faa819

Re: [Qemu-devel] [PATCH v2 01/14] qdict: add qdict_put_null() helper

[Eduardo Habkost]

On Fri, Aug 25, 2017 at 12:59:00PM +0200, Marc-André Lureau wrote:
> A step towards completeness.
> 
> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> Reviewed-by: Markus Armbruster <armbru@redhat.com>

Now replying to the latest version:

Reviewed-by: Eduardo Habkost <ehabkost@redhat.com>

-- 
Eduardo

Re: [Qemu-devel] [PATCH v2 01/14] qdict: add qdict_put_null() helper

[Eric Blake]

[-- Attachment #1: Type: text/plain, Size: 616 bytes --]

On 08/25/2017 05:59 AM, Marc-André Lureau wrote:
> A step towards completeness.
> 
> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> Reviewed-by: Markus Armbruster <armbru@redhat.com>
> ---
>  include/qapi/qmp/qdict.h | 4 +++-
>  target/i386/cpu.c        | 4 ++--
>  2 files changed, 5 insertions(+), 3 deletions(-)

Is it worth touching up scripts/coccinelle/qobject.cocci at the same time?

I guess we don't care about a qlist_append_null() variant?

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 619 bytes --]

Re: [Qemu-devel] [PATCH v2 01/14] qdict: add qdict_put_null() helper

[Markus Armbruster]

Eric Blake <eblake@redhat.com> writes:

> On 08/25/2017 05:59 AM, Marc-André Lureau wrote:
>> A step towards completeness.
>> 
>> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
>> Reviewed-by: Markus Armbruster <armbru@redhat.com>
>> ---
>>  include/qapi/qmp/qdict.h | 4 +++-
>>  target/i386/cpu.c        | 4 ++--
>>  2 files changed, 5 insertions(+), 3 deletions(-)
>
> Is it worth touching up scripts/coccinelle/qobject.cocci at the same time?

Let's keep it up-to-date.  I can do it when I apply.

> I guess we don't care about a qlist_append_null() variant?

Only if we have users.

Re: [Qemu-devel] [PATCH v2 01/14] qdict: add qdict_put_null() helper

[Eric Blake]

[-- Attachment #1: Type: text/plain, Size: 1200 bytes --]

On 08/30/2017 07:01 AM, Markus Armbruster wrote:
> Eric Blake <eblake@redhat.com> writes:
> 
>> On 08/25/2017 05:59 AM, Marc-André Lureau wrote:
>>> A step towards completeness.
>>>
>>> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
>>> Reviewed-by: Markus Armbruster <armbru@redhat.com>
>>> ---
>>>  include/qapi/qmp/qdict.h | 4 +++-
>>>  target/i386/cpu.c        | 4 ++--
>>>  2 files changed, 5 insertions(+), 3 deletions(-)
>>
>> Is it worth touching up scripts/coccinelle/qobject.cocci at the same time?
> 
> Let's keep it up-to-date.  I can do it when I apply.
> 
>> I guess we don't care about a qlist_append_null() variant?
> 
> Only if we have users.

Most (all?) of our qlist_ users tend to be heterogenous; we insert list
members that have the same type. I don't see anyone wanting a list of
all null objects any time soon; about the only reason to support it
would be if we wanted a list of the StrOrNull alternate type, where some
list elements can be null.  But you're right that we don't have to add
it now.

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 619 bytes --]

Re: [Qemu-devel] [PATCH v2 01/14] qdict: add qdict_put_null() helper

[Markus Armbruster]

Markus Armbruster <armbru@redhat.com> writes:

> Eric Blake <eblake@redhat.com> writes:
>
>> On 08/25/2017 05:59 AM, Marc-André Lureau wrote:
>>> A step towards completeness.
>>> 
>>> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
>>> Reviewed-by: Markus Armbruster <armbru@redhat.com>
>>> ---
>>>  include/qapi/qmp/qdict.h | 4 +++-
>>>  target/i386/cpu.c        | 4 ++--
>>>  2 files changed, 5 insertions(+), 3 deletions(-)
>>
>> Is it worth touching up scripts/coccinelle/qobject.cocci at the same time?
>
> Let's keep it up-to-date.  I can do it when I apply.

Squashing in the appended patch.  I verified it produces exactly the
change to target/i386/cpu.c Marc-André did manually, and no others.


diff --git a/scripts/coccinelle/qobject.cocci b/scripts/coccinelle/qobject.cocci
index c518caf7b1..1120eb1a42 100644
--- a/scripts/coccinelle/qobject.cocci
+++ b/scripts/coccinelle/qobject.cocci
@@ -20,6 +20,9 @@ expression Obj, Key, E;
 |
 - qdict_put(Obj, Key, qstring_from_str(E));
 + qdict_put_str(Obj, Key, E);
+|
+- qdict_put(Obj, Key, qnull());
++ qdict_put_null(Obj, Key);
 )
 
 // Use QList macros where they make sense

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

[Marc-André Lureau]

Fix code style issues while at it, to please checkpatch.

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Reviewed-by: Markus Armbruster <armbru@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..280db5064a
--- /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
-- 
2.14.1.146.gd35faa819

[Qemu-devel] [PATCH v2 03/14] qlit: use QLit prefix consistently

[Marc-André Lureau]

Rename from LiteralQ to QLit.

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
---
 include/qapi/qmp/qlit.h | 24 ++++++++++++------------
 qobject/qlit.c          |  4 ++--
 tests/check-qjson.c     | 40 ++++++++++++++++++++--------------------
 3 files changed, 34 insertions(+), 34 deletions(-)

diff --git a/include/qapi/qmp/qlit.h b/include/qapi/qmp/qlit.h
index 280db5064a..a4ad91321b 100644
--- a/include/qapi/qmp/qlit.h
+++ b/include/qapi/qmp/qlit.h
@@ -17,33 +17,33 @@
 #include "qapi-types.h"
 #include "qobject.h"
 
-typedef struct LiteralQDictEntry LiteralQDictEntry;
-typedef struct LiteralQObject LiteralQObject;
+typedef struct QLitDictEntry QLitDictEntry;
+typedef struct QLitObject QLitObject;
 
-struct LiteralQObject {
+struct QLitObject {
     int type;
     union {
         int64_t qnum;
         const char *qstr;
-        LiteralQDictEntry *qdict;
-        LiteralQObject *qlist;
+        QLitDictEntry *qdict;
+        QLitObject *qlist;
     } value;
 };
 
-struct LiteralQDictEntry {
+struct QLitDictEntry {
     const char *key;
-    LiteralQObject value;
+    QLitObject value;
 };
 
 #define QLIT_QNUM(val) \
-    (LiteralQObject){.type = QTYPE_QNUM, .value.qnum = (val)}
+    (QLitObject){.type = QTYPE_QNUM, .value.qnum = (val)}
 #define QLIT_QSTR(val) \
-    (LiteralQObject){.type = QTYPE_QSTRING, .value.qstr = (val)}
+    (QLitObject){.type = QTYPE_QSTRING, .value.qstr = (val)}
 #define QLIT_QDICT(val) \
-    (LiteralQObject){.type = QTYPE_QDICT, .value.qdict = (val)}
+    (QLitObject){.type = QTYPE_QDICT, .value.qdict = (val)}
 #define QLIT_QLIST(val) \
-    (LiteralQObject){.type = QTYPE_QLIST, .value.qlist = (val)}
+    (QLitObject){.type = QTYPE_QLIST, .value.qlist = (val)}
 
-int compare_litqobj_to_qobj(LiteralQObject *lhs, QObject *rhs);
+int compare_litqobj_to_qobj(QLitObject *lhs, QObject *rhs);
 
 #endif /* QLIT_H */
diff --git a/qobject/qlit.c b/qobject/qlit.c
index 5917c3584e..262d64988d 100644
--- a/qobject/qlit.c
+++ b/qobject/qlit.c
@@ -20,7 +20,7 @@
 
 typedef struct QListCompareHelper {
     int index;
-    LiteralQObject *objs;
+    QLitObject *objs;
     int result;
 } QListCompareHelper;
 
@@ -41,7 +41,7 @@ static void compare_helper(QObject *obj, void *opaque)
         compare_litqobj_to_qobj(&helper->objs[helper->index++], obj);
 }
 
-int compare_litqobj_to_qobj(LiteralQObject *lhs, QObject *rhs)
+int compare_litqobj_to_qobj(QLitObject *lhs, QObject *rhs)
 {
     int64_t val;
 
diff --git a/tests/check-qjson.c b/tests/check-qjson.c
index 525f79e60b..82b3681327 100644
--- a/tests/check-qjson.c
+++ b/tests/check-qjson.c
@@ -1065,23 +1065,23 @@ static void simple_dict(void)
     int i;
     struct {
         const char *encoded;
-        LiteralQObject decoded;
+        QLitObject decoded;
     } test_cases[] = {
         {
             .encoded = "{\"foo\": 42, \"bar\": \"hello world\"}",
-            .decoded = QLIT_QDICT(((LiteralQDictEntry[]){
+            .decoded = QLIT_QDICT(((QLitDictEntry[]){
                         { "foo", QLIT_QNUM(42) },
                         { "bar", QLIT_QSTR("hello world") },
                         { }
                     })),
         }, {
             .encoded = "{}",
-            .decoded = QLIT_QDICT(((LiteralQDictEntry[]){
+            .decoded = QLIT_QDICT(((QLitDictEntry[]){
                         { }
                     })),
         }, {
             .encoded = "{\"foo\": 43}",
-            .decoded = QLIT_QDICT(((LiteralQDictEntry[]){
+            .decoded = QLIT_QDICT(((QLitDictEntry[]){
                         { "foo", QLIT_QNUM(43) },
                         { }
                     })),
@@ -1163,11 +1163,11 @@ static void simple_list(void)
     int i;
     struct {
         const char *encoded;
-        LiteralQObject decoded;
+        QLitObject decoded;
     } test_cases[] = {
         {
             .encoded = "[43,42]",
-            .decoded = QLIT_QLIST(((LiteralQObject[]){
+            .decoded = QLIT_QLIST(((QLitObject[]){
                         QLIT_QNUM(43),
                         QLIT_QNUM(42),
                         { }
@@ -1175,21 +1175,21 @@ static void simple_list(void)
         },
         {
             .encoded = "[43]",
-            .decoded = QLIT_QLIST(((LiteralQObject[]){
+            .decoded = QLIT_QLIST(((QLitObject[]){
                         QLIT_QNUM(43),
                         { }
                     })),
         },
         {
             .encoded = "[]",
-            .decoded = QLIT_QLIST(((LiteralQObject[]){
+            .decoded = QLIT_QLIST(((QLitObject[]){
                         { }
                     })),
         },
         {
             .encoded = "[{}]",
-            .decoded = QLIT_QLIST(((LiteralQObject[]){
-                        QLIT_QDICT(((LiteralQDictEntry[]){
+            .decoded = QLIT_QLIST(((QLitObject[]){
+                        QLIT_QDICT(((QLitDictEntry[]){
                                     {},
                                         })),
                         {},
@@ -1220,11 +1220,11 @@ static void simple_whitespace(void)
     int i;
     struct {
         const char *encoded;
-        LiteralQObject decoded;
+        QLitObject decoded;
     } test_cases[] = {
         {
             .encoded = " [ 43 , 42 ]",
-            .decoded = QLIT_QLIST(((LiteralQObject[]){
+            .decoded = QLIT_QLIST(((QLitObject[]){
                         QLIT_QNUM(43),
                         QLIT_QNUM(42),
                         { }
@@ -1232,12 +1232,12 @@ static void simple_whitespace(void)
         },
         {
             .encoded = " [ 43 , { 'h' : 'b' }, [ ], 42 ]",
-            .decoded = QLIT_QLIST(((LiteralQObject[]){
+            .decoded = QLIT_QLIST(((QLitObject[]){
                         QLIT_QNUM(43),
-                        QLIT_QDICT(((LiteralQDictEntry[]){
+                        QLIT_QDICT(((QLitDictEntry[]){
                                     { "h", QLIT_QSTR("b") },
                                     { }})),
-                        QLIT_QLIST(((LiteralQObject[]){
+                        QLIT_QLIST(((QLitObject[]){
                                     { }})),
                         QLIT_QNUM(42),
                         { }
@@ -1245,13 +1245,13 @@ static void simple_whitespace(void)
         },
         {
             .encoded = " [ 43 , { 'h' : 'b' , 'a' : 32 }, [ ], 42 ]",
-            .decoded = QLIT_QLIST(((LiteralQObject[]){
+            .decoded = QLIT_QLIST(((QLitObject[]){
                         QLIT_QNUM(43),
-                        QLIT_QDICT(((LiteralQDictEntry[]){
+                        QLIT_QDICT(((QLitDictEntry[]){
                                     { "h", QLIT_QSTR("b") },
                                     { "a", QLIT_QNUM(32) },
                                     { }})),
-                        QLIT_QLIST(((LiteralQObject[]){
+                        QLIT_QLIST(((QLitObject[]){
                                     { }})),
                         QLIT_QNUM(42),
                         { }
@@ -1282,10 +1282,10 @@ static void simple_varargs(void)
 {
     QObject *embedded_obj;
     QObject *obj;
-    LiteralQObject decoded = QLIT_QLIST(((LiteralQObject[]){
+    QLitObject decoded = QLIT_QLIST(((QLitObject[]){
             QLIT_QNUM(1),
             QLIT_QNUM(2),
-            QLIT_QLIST(((LiteralQObject[]){
+            QLIT_QLIST(((QLitObject[]){
                         QLIT_QNUM(32),
                         QLIT_QNUM(42),
                         {}})),
-- 
2.14.1.146.gd35faa819

[Qemu-devel] [PATCH v2 04/14] qlit: remove compound literals

[Marc-André Lureau]

They are not considered constant expressions in C, producing an error
when compiling a const QLit.

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
 include/qapi/qmp/qlit.h | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/include/qapi/qmp/qlit.h b/include/qapi/qmp/qlit.h
index a4ad91321b..f1d6eed317 100644
--- a/include/qapi/qmp/qlit.h
+++ b/include/qapi/qmp/qlit.h
@@ -36,13 +36,13 @@ struct QLitDictEntry {
 };
 
 #define QLIT_QNUM(val) \
-    (QLitObject){.type = QTYPE_QNUM, .value.qnum = (val)}
+    { .type = QTYPE_QNUM, .value.qnum = (val) }
 #define QLIT_QSTR(val) \
-    (QLitObject){.type = QTYPE_QSTRING, .value.qstr = (val)}
+    { .type = QTYPE_QSTRING, .value.qstr = (val) }
 #define QLIT_QDICT(val) \
-    (QLitObject){.type = QTYPE_QDICT, .value.qdict = (val)}
+    { .type = QTYPE_QDICT, .value.qdict = (val) }
 #define QLIT_QLIST(val) \
-    (QLitObject){.type = QTYPE_QLIST, .value.qlist = (val)}
+    { .type = QTYPE_QLIST, .value.qlist = (val) }
 
 int compare_litqobj_to_qobj(QLitObject *lhs, QObject *rhs);
 
-- 
2.14.1.146.gd35faa819

Re: [Qemu-devel] [PATCH v2 04/14] qlit: remove compound literals

[Markus Armbruster]

Marc-André Lureau <marcandre.lureau@redhat.com> writes:

> They are not considered constant expressions in C, producing an error
> when compiling a const QLit.

A const QLit?  Do you mean a non-const one?

> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> ---
>  include/qapi/qmp/qlit.h | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/include/qapi/qmp/qlit.h b/include/qapi/qmp/qlit.h
> index a4ad91321b..f1d6eed317 100644
> --- a/include/qapi/qmp/qlit.h
> +++ b/include/qapi/qmp/qlit.h
> @@ -36,13 +36,13 @@ struct QLitDictEntry {
>  };
>  
>  #define QLIT_QNUM(val) \
> -    (QLitObject){.type = QTYPE_QNUM, .value.qnum = (val)}
> +    { .type = QTYPE_QNUM, .value.qnum = (val) }
>  #define QLIT_QSTR(val) \
> -    (QLitObject){.type = QTYPE_QSTRING, .value.qstr = (val)}
> +    { .type = QTYPE_QSTRING, .value.qstr = (val) }
>  #define QLIT_QDICT(val) \
> -    (QLitObject){.type = QTYPE_QDICT, .value.qdict = (val)}
> +    { .type = QTYPE_QDICT, .value.qdict = (val) }
>  #define QLIT_QLIST(val) \
> -    (QLitObject){.type = QTYPE_QLIST, .value.qlist = (val)}
> +    { .type = QTYPE_QLIST, .value.qlist = (val) }
>  
>  int compare_litqobj_to_qobj(QLitObject *lhs, QObject *rhs);

Re: [Qemu-devel] [PATCH v2 04/14] qlit: remove compound literals

[Marc-André Lureau]

Hi

----- Original Message -----
> Marc-André Lureau <marcandre.lureau@redhat.com> writes:
> 
> > They are not considered constant expressions in C, producing an error
> > when compiling a const QLit.
> 
> A const QLit?  Do you mean a non-const one?

Really a const QLitObject:


const QLitObject qmp_schema_qlit = QLIT_QLIST(((QLitObject[]) {
             QLIT_QNULL,
             {}
         }));

qmp-introspect.c:17:63: error: initializer element is not constant
  const QLitObject qmp_schema_qlit = QLIT_QLIST(((QLitObject[]) {
                                                                ^
Removing the "compound literals" fixes this error.

We may want to include it in the commit message, but I think it lacks a bit of "C standard" explanation. I think it is something like "compound literals" are not const. But then why does it work with (QLitObject[]) ? :)


> 
> > Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> > ---
> >  include/qapi/qmp/qlit.h | 8 ++++----
> >  1 file changed, 4 insertions(+), 4 deletions(-)
> >
> > diff --git a/include/qapi/qmp/qlit.h b/include/qapi/qmp/qlit.h
> > index a4ad91321b..f1d6eed317 100644
> > --- a/include/qapi/qmp/qlit.h
> > +++ b/include/qapi/qmp/qlit.h
> > @@ -36,13 +36,13 @@ struct QLitDictEntry {
> >  };
> >  
> >  #define QLIT_QNUM(val) \
> > -    (QLitObject){.type = QTYPE_QNUM, .value.qnum = (val)}
> > +    { .type = QTYPE_QNUM, .value.qnum = (val) }
> >  #define QLIT_QSTR(val) \
> > -    (QLitObject){.type = QTYPE_QSTRING, .value.qstr = (val)}
> > +    { .type = QTYPE_QSTRING, .value.qstr = (val) }
> >  #define QLIT_QDICT(val) \
> > -    (QLitObject){.type = QTYPE_QDICT, .value.qdict = (val)}
> > +    { .type = QTYPE_QDICT, .value.qdict = (val) }
> >  #define QLIT_QLIST(val) \
> > -    (QLitObject){.type = QTYPE_QLIST, .value.qlist = (val)}
> > +    { .type = QTYPE_QLIST, .value.qlist = (val) }
> >  
> >  int compare_litqobj_to_qobj(QLitObject *lhs, QObject *rhs);
> 

Re: [Qemu-devel] [PATCH v2 04/14] qlit: remove compound literals

[Markus Armbruster]

Marc-André Lureau <marcandre.lureau@redhat.com> writes:

> Hi
>
> ----- Original Message -----
>> Marc-André Lureau <marcandre.lureau@redhat.com> writes:
>> 
>> > They are not considered constant expressions in C, producing an error
>> > when compiling a const QLit.
>> 
>> A const QLit?  Do you mean a non-const one?
>
> Really a const QLitObject:
>
>
> const QLitObject qmp_schema_qlit = QLIT_QLIST(((QLitObject[]) {
>              QLIT_QNULL,
>              {}
>          }));
>
> qmp-introspect.c:17:63: error: initializer element is not constant
>   const QLitObject qmp_schema_qlit = QLIT_QLIST(((QLitObject[]) {
>                                                                 ^
> Removing the "compound literals" fixes this error.

Does QLIT_QLIST(((const QLitObject[]) { ... } work?

> We may want to include it in the commit message, but I think it lacks a bit of "C standard" explanation. I think it is something like "compound literals" are not const. But then why does it work with (QLitObject[]) ? :)

Re: [Qemu-devel] [PATCH v2 04/14] qlit: remove compound literals

[Marc-André Lureau]

Hi

----- Original Message -----
> Marc-André Lureau <marcandre.lureau@redhat.com> writes:
> 
> > Hi
> >
> > ----- Original Message -----
> >> Marc-André Lureau <marcandre.lureau@redhat.com> writes:
> >> 
> >> > They are not considered constant expressions in C, producing an error
> >> > when compiling a const QLit.
> >> 
> >> A const QLit?  Do you mean a non-const one?
> >
> > Really a const QLitObject:
> >
> >
> > const QLitObject qmp_schema_qlit = QLIT_QLIST(((QLitObject[]) {
> >              QLIT_QNULL,
> >              {}
> >          }));
> >
> > qmp-introspect.c:17:63: error: initializer element is not constant
> >   const QLitObject qmp_schema_qlit = QLIT_QLIST(((QLitObject[]) {
> >                                                                 ^
> > Removing the "compound literals" fixes this error.
> 
> Does QLIT_QLIST(((const QLitObject[]) { ... } work?

No. Even if I put "const" all over the place (in member, in compound type etc).

Give it a try, see if you can make it const, I am out of luck.

Re: [Qemu-devel] [PATCH v2 04/14] qlit: remove compound literals

[Markus Armbruster]

Marc-André Lureau <marcandre.lureau@redhat.com> writes:

> Hi
>
> ----- Original Message -----
>> Marc-André Lureau <marcandre.lureau@redhat.com> writes:
>> 
>> > Hi
>> >
>> > ----- Original Message -----
>> >> Marc-André Lureau <marcandre.lureau@redhat.com> writes:
>> >> 
>> >> > They are not considered constant expressions in C, producing an error
>> >> > when compiling a const QLit.
>> >> 
>> >> A const QLit?  Do you mean a non-const one?
>> >
>> > Really a const QLitObject:
>> >
>> >
>> > const QLitObject qmp_schema_qlit = QLIT_QLIST(((QLitObject[]) {
>> >              QLIT_QNULL,
>> >              {}
>> >          }));
>> >
>> > qmp-introspect.c:17:63: error: initializer element is not constant
>> >   const QLitObject qmp_schema_qlit = QLIT_QLIST(((QLitObject[]) {
>> >                                                                 ^
>> > Removing the "compound literals" fixes this error.
>> 
>> Does QLIT_QLIST(((const QLitObject[]) { ... } work?
>
> No. Even if I put "const" all over the place (in member, in compound type etc).
>
> Give it a try, see if you can make it const, I am out of luck.

The commit message's explanation is wrong.  This isn't about const at
all, it's about "constant expressions", which are something else
entirely.

For what it's worth, clang is cool with the compound literals.  On
Fedora 26 with a minimized test case (appended):

    $ clang -c -g -O -Wall compound-lit.c
    $ gcc -c -g -O -Wall compound-lit.c
    compound-lit.c:30:37: error: initializer element is not constant
         .value.qdict = (QLitDictEntry[]){
                                         ^
    compound-lit.c:30:37: note: (near initialization for ‘(anonymous).value’)

GCC bug or not?  A search of the GCC Bugzilla finds
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=71713

Copying a few notorious language lawyers^W^W^Wtrusted advisers.

Even if this turns out to be a gcc bug, we'll have to work around it.
But the work-around needs a comment then.

In any case, the commit message needs fixing.



enum {
    QTYPE_NONE, QTYPE_QSTRING, QTYPE_QDICT,
};

typedef struct QLitDictEntry QLitDictEntry;
typedef struct QLitObject QLitObject;

struct QLitObject {
    int type;
    union {
        const char *qstr;
        QLitDictEntry *qdict;
    } value;
};

struct QLitDictEntry {
    const char *key;
    QLitObject value;
};

QLitObject qlit1 = (QLitObject){
    .type = QTYPE_QDICT,
    .value.qdict = (QLitDictEntry[]){
	{ "foo", {} },
	{}
    }};

QLitObject qlit2 = (QLitObject){
    .type = QTYPE_QDICT,
    .value.qdict = (QLitDictEntry[]){
	{ "foo", (QLitObject){} },
	{}
    }};

Re: [Qemu-devel] [PATCH v2 04/14] qlit: remove compound literals

[Marc-André Lureau]

Hi

On Thu, Aug 31, 2017 at 10:43 AM Markus Armbruster <armbru@redhat.com>
wrote:

> Marc-André Lureau <marcandre.lureau@redhat.com> writes:
>
> > Hi
> >
> > ----- Original Message -----
> >> Marc-André Lureau <marcandre.lureau@redhat.com> writes:
> >>
> >> > Hi
> >> >
> >> > ----- Original Message -----
> >> >> Marc-André Lureau <marcandre.lureau@redhat.com> writes:
> >> >>
> >> >> > They are not considered constant expressions in C, producing an
> error
> >> >> > when compiling a const QLit.
> >> >>
> >> >> A const QLit?  Do you mean a non-const one?
> >> >
> >> > Really a const QLitObject:
> >> >
> >> >
> >> > const QLitObject qmp_schema_qlit = QLIT_QLIST(((QLitObject[]) {
> >> >              QLIT_QNULL,
> >> >              {}
> >> >          }));
> >> >
> >> > qmp-introspect.c:17:63: error: initializer element is not constant
> >> >   const QLitObject qmp_schema_qlit = QLIT_QLIST(((QLitObject[]) {
> >> >                                                                 ^
> >> > Removing the "compound literals" fixes this error.
> >>
> >> Does QLIT_QLIST(((const QLitObject[]) { ... } work?
> >
> > No. Even if I put "const" all over the place (in member, in compound
> type etc).
> >
> > Give it a try, see if you can make it const, I am out of luck.
>
> The commit message's explanation is wrong.  This isn't about const at
> all, it's about "constant expressions", which are something else
> entirely.
>

The point was that declaring a non const QLit with those "compound
literals" worked vs with const.


> For what it's worth, clang is cool with the compound literals.  On
> Fedora 26 with a minimized test case (appended):
>
>     $ clang -c -g -O -Wall compound-lit.c
>     $ gcc -c -g -O -Wall compound-lit.c
>     compound-lit.c:30:37: error: initializer element is not constant
>          .value.qdict = (QLitDictEntry[]){
>                                          ^
>     compound-lit.c:30:37: note: (near initialization for
> ‘(anonymous).value’)
>
> GCC bug or not?  A search of the GCC Bugzilla finds
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=71713
>
> Copying a few notorious language lawyers^W^W^Wtrusted advisers.
>
> Even if this turns out to be a gcc bug, we'll have to work around it.
> But the work-around needs a comment then.
>
> In any case, the commit message needs fixing.
>

What about adapting the bug comment:

qlit: remove compound literals

A compound literal (i.e., "(struct Str1){1}"), is not a constant
expression, and so it cannot be used to initialize an object with static
storage duration.

$ gcc -c -g -O -Wall compound-lit.c
    compound-lit.c:30:37: error: initializer element is not constant
         .value.qdict = (QLitDictEntry[]){
                                         ^
    compound-lit.c:30:37: note: (near initialization for
‘(anonymous).value’)

clang accepts it. In some cases, gcc accepts compound literals as
initializer, but not in this nested case. There is a gcc bug about it:
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=71713.

Feel free to adapt on commit

thanks


>
>
> enum {
>     QTYPE_NONE, QTYPE_QSTRING, QTYPE_QDICT,
> };
>
> typedef struct QLitDictEntry QLitDictEntry;
> typedef struct QLitObject QLitObject;
>
> struct QLitObject {
>     int type;
>     union {
>         const char *qstr;
>         QLitDictEntry *qdict;
>     } value;
> };
>
> struct QLitDictEntry {
>     const char *key;
>     QLitObject value;
> };
>
> QLitObject qlit1 = (QLitObject){
>     .type = QTYPE_QDICT,
>     .value.qdict = (QLitDictEntry[]){
>         { "foo", {} },
>         {}
>     }};
>
> QLitObject qlit2 = (QLitObject){
>     .type = QTYPE_QDICT,
>     .value.qdict = (QLitDictEntry[]){
>         { "foo", (QLitObject){} },
>         {}
>     }};
>
> --
Marc-André Lureau

Re: [Qemu-devel] [PATCH v2 04/14] qlit: remove compound literals

[Markus Armbruster]

Marc-André Lureau <marcandre.lureau@gmail.com> writes:

> Hi
>
> On Thu, Aug 31, 2017 at 10:43 AM Markus Armbruster <armbru@redhat.com>
> wrote:
>
>> Marc-André Lureau <marcandre.lureau@redhat.com> writes:
>>
>> > Hi
>> >
>> > ----- Original Message -----
>> >> Marc-André Lureau <marcandre.lureau@redhat.com> writes:
>> >>
>> >> > Hi
>> >> >
>> >> > ----- Original Message -----
>> >> >> Marc-André Lureau <marcandre.lureau@redhat.com> writes:
>> >> >>
>> >> >> > They are not considered constant expressions in C, producing an
>> error
>> >> >> > when compiling a const QLit.
>> >> >>
>> >> >> A const QLit?  Do you mean a non-const one?
>> >> >
>> >> > Really a const QLitObject:
>> >> >
>> >> >
>> >> > const QLitObject qmp_schema_qlit = QLIT_QLIST(((QLitObject[]) {
>> >> >              QLIT_QNULL,
>> >> >              {}
>> >> >          }));
>> >> >
>> >> > qmp-introspect.c:17:63: error: initializer element is not constant
>> >> >   const QLitObject qmp_schema_qlit = QLIT_QLIST(((QLitObject[]) {
>> >> >                                                                 ^
>> >> > Removing the "compound literals" fixes this error.
>> >>
>> >> Does QLIT_QLIST(((const QLitObject[]) { ... } work?
>> >
>> > No. Even if I put "const" all over the place (in member, in compound
>> type etc).
>> >
>> > Give it a try, see if you can make it const, I am out of luck.
>>
>> The commit message's explanation is wrong.  This isn't about const at
>> all, it's about "constant expressions", which are something else
>> entirely.
>>
>
> The point was that declaring a non const QLit with those "compound
> literals" worked vs with const.

The keyword const is a red herring here, and that's precisely what's
wrong with the commit message.  The minimized test case demonstrates the
problem without const.

>> For what it's worth, clang is cool with the compound literals.  On
>> Fedora 26 with a minimized test case (appended):
>>
>>     $ clang -c -g -O -Wall compound-lit.c
>>     $ gcc -c -g -O -Wall compound-lit.c
>>     compound-lit.c:30:37: error: initializer element is not constant
>>          .value.qdict = (QLitDictEntry[]){
>>                                          ^
>>     compound-lit.c:30:37: note: (near initialization for
>> ‘(anonymous).value’)
>>
>> GCC bug or not?  A search of the GCC Bugzilla finds
>> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=71713
>>
>> Copying a few notorious language lawyers^W^W^Wtrusted advisers.
>>
>> Even if this turns out to be a gcc bug, we'll have to work around it.
>> But the work-around needs a comment then.
>>
>> In any case, the commit message needs fixing.
>>
>
> What about adapting the bug comment:
>
> qlit: remove compound literals
>
> A compound literal (i.e., "(struct Str1){1}"), is not a constant
> expression, and so it cannot be used to initialize an object with static
> storage duration.

That's better.

It's weird that a compound literal isn't a constant expression, but
arguing about it here won't make gcc treat it as one.

> $ gcc -c -g -O -Wall compound-lit.c
>     compound-lit.c:30:37: error: initializer element is not constant
>          .value.qdict = (QLitDictEntry[]){
>                                          ^
>     compound-lit.c:30:37: note: (near initialization for
> ‘(anonymous).value’)
>
> clang accepts it. In some cases, gcc accepts compound literals as
> initializer, but not in this nested case. There is a gcc bug about it:
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=71713.
>
> Feel free to adapt on commit

Using this:

    qlit: Change compound literals to initializers

    The QLIT_QFOO() macros expand into compound literals.  Sadly, gcc
    doesn't recognizes these as constant expressions (clang does), which
    makes the macros useless for initializing objects with static storage
    duration.

    There is a gcc bug about it:
    https://gcc.gnu.org/bugzilla/show_bug.cgi?id=71713

    Change the macros to expand into initializers.

Avoids passing judgement on "bug or no bug", and avoids referring to the
compount-lit.c example without actually including it.

I might still add a comment.

Re: [Qemu-devel] [PATCH v2 04/14] qlit: remove compound literals

[Laszlo Ersek]

On 08/31/17 10:42, Markus Armbruster wrote:
> Marc-André Lureau <marcandre.lureau@redhat.com> writes:
> 
>> Hi
>>
>> ----- Original Message -----
>>> Marc-André Lureau <marcandre.lureau@redhat.com> writes:
>>>
>>>> Hi
>>>>
>>>> ----- Original Message -----
>>>>> Marc-André Lureau <marcandre.lureau@redhat.com> writes:
>>>>>
>>>>>> They are not considered constant expressions in C, producing an error
>>>>>> when compiling a const QLit.
>>>>>
>>>>> A const QLit?  Do you mean a non-const one?
>>>>
>>>> Really a const QLitObject:
>>>>
>>>>
>>>> const QLitObject qmp_schema_qlit = QLIT_QLIST(((QLitObject[]) {
>>>>              QLIT_QNULL,
>>>>              {}
>>>>          }));
>>>>
>>>> qmp-introspect.c:17:63: error: initializer element is not constant
>>>>   const QLitObject qmp_schema_qlit = QLIT_QLIST(((QLitObject[]) {
>>>>                                                                 ^
>>>> Removing the "compound literals" fixes this error.
>>>
>>> Does QLIT_QLIST(((const QLitObject[]) { ... } work?
>>
>> No. Even if I put "const" all over the place (in member, in compound type etc).
>>
>> Give it a try, see if you can make it const, I am out of luck.
> 
> The commit message's explanation is wrong.  This isn't about const at
> all, it's about "constant expressions", which are something else
> entirely.
> 
> For what it's worth, clang is cool with the compound literals.  On
> Fedora 26 with a minimized test case (appended):
> 
>     $ clang -c -g -O -Wall compound-lit.c
>     $ gcc -c -g -O -Wall compound-lit.c
>     compound-lit.c:30:37: error: initializer element is not constant
>          .value.qdict = (QLitDictEntry[]){
>                                          ^
>     compound-lit.c:30:37: note: (near initialization for ‘(anonymous).value’)
> 
> GCC bug or not?  A search of the GCC Bugzilla finds
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=71713
> 
> Copying a few notorious language lawyers^W^W^Wtrusted advisers.
> 
> Even if this turns out to be a gcc bug, we'll have to work around it.
> But the work-around needs a comment then.
> 
> In any case, the commit message needs fixing.
> 
> 
> 
> enum {
>     QTYPE_NONE, QTYPE_QSTRING, QTYPE_QDICT,
> };
> 
> typedef struct QLitDictEntry QLitDictEntry;
> typedef struct QLitObject QLitObject;
> 
> struct QLitObject {
>     int type;
>     union {
>         const char *qstr;
>         QLitDictEntry *qdict;
>     } value;
> };
> 
> struct QLitDictEntry {
>     const char *key;
>     QLitObject value;
> };
> 
> QLitObject qlit1 = (QLitObject){
>     .type = QTYPE_QDICT,
>     .value.qdict = (QLitDictEntry[]){
> 	{ "foo", {} },
> 	{}
>     }};
> 
> QLitObject qlit2 = (QLitObject){
>     .type = QTYPE_QDICT,
>     .value.qdict = (QLitDictEntry[]){
> 	{ "foo", (QLitObject){} },
> 	{}
>     }};
> 

(1) When discussing standards conformance, please drop the {} construct;
it is a GNUism. Replacing it with { 0 } works in all contexts, and
conforms to the standard. (Not trying to be pedantic here, but it does
elicit extra warnings from my gcc command line

gcc -std=c99 -pedantic -Wall -Wextra -fsyntax-only


(2) Let's see what the standard says:

  6.5.2.5 Compound literals
  Constraints
  3 If the compound literal occurs outside the body of a function, the
    initializer list shall consist of constant expressions.

In the initialization of "qlit1", one element of the initializer list
(namely for .value.qdict) is

[1] (QLitDictEntry[]) {
      { "foo", { 0 } },
      { 0 }
    }

Is this a constant expression?

  6.6 Constant expressions
  7 More latitude is permitted for constant expressions in initializers.
    Such a constant expression shall be, or evaluate to, one of the
    following:
    - an arithmetic constant expression,
    - a null pointer constant,
    - an address constant, or
    - an address constant for an object type plus or minus an integer
      constant expression.

Now, is [1] an address constant?

  6.6 Constant expressions
  9 An address constant is a null pointer, a pointer to an lvalue
    designating an object of static storage duration, or a pointer to a
    function designator; it shall be created explicitly using the unary
    & operator or an integer constant cast to pointer type, or
    implicitly by the use of an expression of array or function type.
    The array-subscript [] and member-access . and -> operators, the
    address & and indirection * unary operators, and pointer casts may
    be used in the creation of an address constant, but the value of an
    object shall not be accessed by use of these operators.

"expression of array [...] type" applies; question is:
- is [1] an lvalue designating an object of static storage duration?

  6.5.2.5 Compound literals
  Semantics
  5 If the type name specifies an array of unknown size, the size is
    determined by the initializer list as specified in 6.7.8, and the
    type of the compound literal is that of the completed array type.
    Otherwise (when the type name specifies an object type), the type
    of the compound literal is that specified by the type name. In
    either case, the result is an lvalue.

So, an lvalue [1] is.

  6 The value of the compound literal is that of an unnamed object
    initialized by the initializer list. If the compound literal occurs
    outside the body of a function, the object has static storage
    duration; otherwise, it has automatic storage duration associated
    with the enclosing block.

Static storage duration is therefore also proven; the initializer [1]
that we provide for ".value.qdict" *is* a constant expression.


(3) However, on my side at least -- RHEL-7.4 --, the initializer for
".value.qdict" is not what gcc complains about, in the initialization of
"qlit1"!

The problem is the *outer* compound literal. *That* is indeed not a
constant expression; if you review 6.6p7 above, it does not fit any of
the allowed cases.

However, the outer compound literal doesn't buy us anything! If you
change the code like this, it compiles without a hitch:

--- xx.c        2017-08-31 17:23:05.145481557 +0200
+++ yy.c        2017-08-31 17:25:14.839088894 +0200
@@ -18,7 +18,7 @@
     QLitObject value;
 };

-QLitObject qlit1 = (QLitObject){
+QLitObject qlit1 = {
     .type = QTYPE_QDICT,
     .value.qdict = (QLitDictEntry[]){
       { "foo", { 0 } },


(I ignored "qlit2" for this discussion.)

Thanks
Laszlo

Re: [Qemu-devel] [PATCH v2 04/14] qlit: remove compound literals

[Markus Armbruster]

Laszlo Ersek <lersek@redhat.com> writes:

> On 08/31/17 10:42, Markus Armbruster wrote:
>> Marc-André Lureau <marcandre.lureau@redhat.com> writes:
>> 
>>> Hi
>>>
>>> ----- Original Message -----
>>>> Marc-André Lureau <marcandre.lureau@redhat.com> writes:
>>>>
>>>>> Hi
>>>>>
>>>>> ----- Original Message -----
>>>>>> Marc-André Lureau <marcandre.lureau@redhat.com> writes:
>>>>>>
>>>>>>> They are not considered constant expressions in C, producing an error
>>>>>>> when compiling a const QLit.
>>>>>>
>>>>>> A const QLit?  Do you mean a non-const one?
>>>>>
>>>>> Really a const QLitObject:
>>>>>
>>>>>
>>>>> const QLitObject qmp_schema_qlit = QLIT_QLIST(((QLitObject[]) {
>>>>>              QLIT_QNULL,
>>>>>              {}
>>>>>          }));
>>>>>
>>>>> qmp-introspect.c:17:63: error: initializer element is not constant
>>>>>   const QLitObject qmp_schema_qlit = QLIT_QLIST(((QLitObject[]) {
>>>>>                                                                 ^
>>>>> Removing the "compound literals" fixes this error.
>>>>
>>>> Does QLIT_QLIST(((const QLitObject[]) { ... } work?
>>>
>>> No. Even if I put "const" all over the place (in member, in compound type etc).
>>>
>>> Give it a try, see if you can make it const, I am out of luck.
>> 
>> The commit message's explanation is wrong.  This isn't about const at
>> all, it's about "constant expressions", which are something else
>> entirely.
>> 
>> For what it's worth, clang is cool with the compound literals.  On
>> Fedora 26 with a minimized test case (appended):
>> 
>>     $ clang -c -g -O -Wall compound-lit.c
>>     $ gcc -c -g -O -Wall compound-lit.c
>>     compound-lit.c:30:37: error: initializer element is not constant
>>          .value.qdict = (QLitDictEntry[]){
>>                                          ^
>>     compound-lit.c:30:37: note: (near initialization for ‘(anonymous).value’)
>> 
>> GCC bug or not?  A search of the GCC Bugzilla finds
>> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=71713
>> 
>> Copying a few notorious language lawyers^W^W^Wtrusted advisers.
>> 
>> Even if this turns out to be a gcc bug, we'll have to work around it.
>> But the work-around needs a comment then.
>> 
>> In any case, the commit message needs fixing.
>> 
>> 
>> 
>> enum {
>>     QTYPE_NONE, QTYPE_QSTRING, QTYPE_QDICT,
>> };
>> 
>> typedef struct QLitDictEntry QLitDictEntry;
>> typedef struct QLitObject QLitObject;
>> 
>> struct QLitObject {
>>     int type;
>>     union {
>>         const char *qstr;
>>         QLitDictEntry *qdict;
>>     } value;
>> };
>> 
>> struct QLitDictEntry {
>>     const char *key;
>>     QLitObject value;
>> };
>> 
>> QLitObject qlit1 = (QLitObject){
>>     .type = QTYPE_QDICT,
>>     .value.qdict = (QLitDictEntry[]){
>> 	{ "foo", {} },
>> 	{}
>>     }};
>> 
>> QLitObject qlit2 = (QLitObject){
>>     .type = QTYPE_QDICT,
>>     .value.qdict = (QLitDictEntry[]){
>> 	{ "foo", (QLitObject){} },
>> 	{}
>>     }};
>> 
>
> (1) When discussing standards conformance, please drop the {} construct;
> it is a GNUism. Replacing it with { 0 } works in all contexts, and
> conforms to the standard. (Not trying to be pedantic here, but it does
> elicit extra warnings from my gcc command line
>
> gcc -std=c99 -pedantic -Wall -Wextra -fsyntax-only
>
>
> (2) Let's see what the standard says:
>
>   6.5.2.5 Compound literals
>   Constraints
>   3 If the compound literal occurs outside the body of a function, the
>     initializer list shall consist of constant expressions.
>
> In the initialization of "qlit1", one element of the initializer list
> (namely for .value.qdict) is
>
> [1] (QLitDictEntry[]) {
>       { "foo", { 0 } },
>       { 0 }
>     }
>
> Is this a constant expression?
>
>   6.6 Constant expressions
>   7 More latitude is permitted for constant expressions in initializers.
>     Such a constant expression shall be, or evaluate to, one of the
>     following:
>     - an arithmetic constant expression,
>     - a null pointer constant,
>     - an address constant, or
>     - an address constant for an object type plus or minus an integer
>       constant expression.
>
> Now, is [1] an address constant?
>
>   6.6 Constant expressions
>   9 An address constant is a null pointer, a pointer to an lvalue
>     designating an object of static storage duration, or a pointer to a
>     function designator; it shall be created explicitly using the unary
>     & operator or an integer constant cast to pointer type, or
>     implicitly by the use of an expression of array or function type.
>     The array-subscript [] and member-access . and -> operators, the
>     address & and indirection * unary operators, and pointer casts may
>     be used in the creation of an address constant, but the value of an
>     object shall not be accessed by use of these operators.
>
> "expression of array [...] type" applies; question is:
> - is [1] an lvalue designating an object of static storage duration?
>
>   6.5.2.5 Compound literals
>   Semantics
>   5 If the type name specifies an array of unknown size, the size is
>     determined by the initializer list as specified in 6.7.8, and the
>     type of the compound literal is that of the completed array type.
>     Otherwise (when the type name specifies an object type), the type
>     of the compound literal is that specified by the type name. In
>     either case, the result is an lvalue.
>
> So, an lvalue [1] is.
>
>   6 The value of the compound literal is that of an unnamed object
>     initialized by the initializer list. If the compound literal occurs
>     outside the body of a function, the object has static storage
>     duration; otherwise, it has automatic storage duration associated
>     with the enclosing block.
>
> Static storage duration is therefore also proven; the initializer [1]
> that we provide for ".value.qdict" *is* a constant expression.
>
>
> (3) However, on my side at least -- RHEL-7.4 --, the initializer for
> ".value.qdict" is not what gcc complains about, in the initialization of
> "qlit1"!
>
> The problem is the *outer* compound literal. *That* is indeed not a
> constant expression; if you review 6.6p7 above, it does not fit any of
> the allowed cases.

In other words, you can have constant expressions of arithmetic and
pointer type (which includes arrays), but not of struct or union type.
Sad.

> However, the outer compound literal doesn't buy us anything! If you
> change the code like this, it compiles without a hitch:
>
> --- xx.c        2017-08-31 17:23:05.145481557 +0200
> +++ yy.c        2017-08-31 17:25:14.839088894 +0200
> @@ -18,7 +18,7 @@
>      QLitObject value;
>  };
>
> -QLitObject qlit1 = (QLitObject){
> +QLitObject qlit1 = {
>      .type = QTYPE_QDICT,
>      .value.qdict = (QLitDictEntry[]){
>        { "foo", { 0 } },
>
>
> (I ignored "qlit2" for this discussion.)

Yes.

I figure the original author chose compound literals over traditional
initializers for their nicely explicit typing.  Sadly, they turn out to
be useless for initializing struct and union types of static storage
duration.  To make the macros usable there, we have to give up the
explicit typing.  While that's sad for the C lanaguage, it's no biggie
for us.  But I wanted to *understand* what we're doing, so I can fix up
the commit message without making a fool out of myself :)

Thanks for your help, László!

[Qemu-devel] [PATCH v2 05/14] qlit: rename compare_litqobj_to_qobj() to qlit_equal_qobject()

[Marc-André Lureau]

compare_litqobj_to_qobj() lacks a qlit_ prefix.  Moreover, "compare"
suggests -1, 0, +1 for less than, equal and greater than.  The
function actually returns non-zero for equal, zero for unequal.
Rename to qlit_equal_qobject().

Its return type will be cleaned up in the next patch.

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
---
 include/qapi/qmp/qlit.h |  2 +-
 qobject/qlit.c          |  6 +++---
 tests/check-qjson.c     | 14 +++++++-------
 3 files changed, 11 insertions(+), 11 deletions(-)

diff --git a/include/qapi/qmp/qlit.h b/include/qapi/qmp/qlit.h
index f1d6eed317..5a180477c8 100644
--- a/include/qapi/qmp/qlit.h
+++ b/include/qapi/qmp/qlit.h
@@ -44,6 +44,6 @@ struct QLitDictEntry {
 #define QLIT_QLIST(val) \
     { .type = QTYPE_QLIST, .value.qlist = (val) }
 
-int compare_litqobj_to_qobj(QLitObject *lhs, QObject *rhs);
+int qlit_equal_qobject(QLitObject *lhs, QObject *rhs);
 
 #endif /* QLIT_H */
diff --git a/qobject/qlit.c b/qobject/qlit.c
index 262d64988d..0c4101898d 100644
--- a/qobject/qlit.c
+++ b/qobject/qlit.c
@@ -38,10 +38,10 @@ static void compare_helper(QObject *obj, void *opaque)
     }
 
     helper->result =
-        compare_litqobj_to_qobj(&helper->objs[helper->index++], obj);
+        qlit_equal_qobject(&helper->objs[helper->index++], obj);
 }
 
-int compare_litqobj_to_qobj(QLitObject *lhs, QObject *rhs)
+int qlit_equal_qobject(QLitObject *lhs, QObject *rhs)
 {
     int64_t val;
 
@@ -63,7 +63,7 @@ int compare_litqobj_to_qobj(QLitObject *lhs, QObject *rhs)
             QObject *obj = qdict_get(qobject_to_qdict(rhs),
                                      lhs->value.qdict[i].key);
 
-            if (!compare_litqobj_to_qobj(&lhs->value.qdict[i].value, obj)) {
+            if (!qlit_equal_qobject(&lhs->value.qdict[i].value, obj)) {
                 return 0;
             }
         }
diff --git a/tests/check-qjson.c b/tests/check-qjson.c
index 82b3681327..e5ca273cbc 100644
--- a/tests/check-qjson.c
+++ b/tests/check-qjson.c
@@ -1094,13 +1094,13 @@ static void simple_dict(void)
         QString *str;
 
         obj = qobject_from_json(test_cases[i].encoded, &error_abort);
-        g_assert(compare_litqobj_to_qobj(&test_cases[i].decoded, obj) == 1);
+        g_assert(qlit_equal_qobject(&test_cases[i].decoded, obj) == 1);
 
         str = qobject_to_json(obj);
         qobject_decref(obj);
 
         obj = qobject_from_json(qstring_get_str(str), &error_abort);
-        g_assert(compare_litqobj_to_qobj(&test_cases[i].decoded, obj) == 1);
+        g_assert(qlit_equal_qobject(&test_cases[i].decoded, obj) == 1);
         qobject_decref(obj);
         QDECREF(str);
     }
@@ -1203,13 +1203,13 @@ static void simple_list(void)
         QString *str;
 
         obj = qobject_from_json(test_cases[i].encoded, &error_abort);
-        g_assert(compare_litqobj_to_qobj(&test_cases[i].decoded, obj) == 1);
+        g_assert(qlit_equal_qobject(&test_cases[i].decoded, obj) == 1);
 
         str = qobject_to_json(obj);
         qobject_decref(obj);
 
         obj = qobject_from_json(qstring_get_str(str), &error_abort);
-        g_assert(compare_litqobj_to_qobj(&test_cases[i].decoded, obj) == 1);
+        g_assert(qlit_equal_qobject(&test_cases[i].decoded, obj) == 1);
         qobject_decref(obj);
         QDECREF(str);
     }
@@ -1265,13 +1265,13 @@ static void simple_whitespace(void)
         QString *str;
 
         obj = qobject_from_json(test_cases[i].encoded, &error_abort);
-        g_assert(compare_litqobj_to_qobj(&test_cases[i].decoded, obj) == 1);
+        g_assert(qlit_equal_qobject(&test_cases[i].decoded, obj) == 1);
 
         str = qobject_to_json(obj);
         qobject_decref(obj);
 
         obj = qobject_from_json(qstring_get_str(str), &error_abort);
-        g_assert(compare_litqobj_to_qobj(&test_cases[i].decoded, obj) == 1);
+        g_assert(qlit_equal_qobject(&test_cases[i].decoded, obj) == 1);
 
         qobject_decref(obj);
         QDECREF(str);
@@ -1295,7 +1295,7 @@ static void simple_varargs(void)
     g_assert(embedded_obj != NULL);
 
     obj = qobject_from_jsonf("[%d, 2, %p]", 1, embedded_obj);
-    g_assert(compare_litqobj_to_qobj(&decoded, obj) == 1);
+    g_assert(qlit_equal_qobject(&decoded, obj) == 1);
 
     qobject_decref(obj);
 }
-- 
2.14.1.146.gd35faa819

[Qemu-devel] [PATCH v2 06/14] qlit: make qlit_equal_qobject return a bool

[Marc-André Lureau]

Make it more obvious about the expected return values.

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
---
 include/qapi/qmp/qlit.h |  2 +-
 qobject/qlit.c          | 18 +++++++++---------
 tests/check-qjson.c     | 14 +++++++-------
 3 files changed, 17 insertions(+), 17 deletions(-)

diff --git a/include/qapi/qmp/qlit.h b/include/qapi/qmp/qlit.h
index 5a180477c8..35aabbdc9f 100644
--- a/include/qapi/qmp/qlit.h
+++ b/include/qapi/qmp/qlit.h
@@ -44,6 +44,6 @@ struct QLitDictEntry {
 #define QLIT_QLIST(val) \
     { .type = QTYPE_QLIST, .value.qlist = (val) }
 
-int qlit_equal_qobject(QLitObject *lhs, QObject *rhs);
+bool qlit_equal_qobject(QLitObject *lhs, QObject *rhs);
 
 #endif /* QLIT_H */
diff --git a/qobject/qlit.c b/qobject/qlit.c
index 0c4101898d..a2975bef3f 100644
--- a/qobject/qlit.c
+++ b/qobject/qlit.c
@@ -21,19 +21,19 @@
 typedef struct QListCompareHelper {
     int index;
     QLitObject *objs;
-    int result;
+    bool result;
 } QListCompareHelper;
 
 static void compare_helper(QObject *obj, void *opaque)
 {
     QListCompareHelper *helper = opaque;
 
-    if (helper->result == 0) {
+    if (!helper->result) {
         return;
     }
 
     if (helper->objs[helper->index].type == QTYPE_NONE) {
-        helper->result = 0;
+        helper->result = false;
         return;
     }
 
@@ -41,12 +41,12 @@ static void compare_helper(QObject *obj, void *opaque)
         qlit_equal_qobject(&helper->objs[helper->index++], obj);
 }
 
-int qlit_equal_qobject(QLitObject *lhs, QObject *rhs)
+bool qlit_equal_qobject(QLitObject *lhs, QObject *rhs)
 {
     int64_t val;
 
     if (!rhs || lhs->type != qobject_type(rhs)) {
-        return 0;
+        return false;
     }
 
     switch (lhs->type) {
@@ -64,18 +64,18 @@ int qlit_equal_qobject(QLitObject *lhs, QObject *rhs)
                                      lhs->value.qdict[i].key);
 
             if (!qlit_equal_qobject(&lhs->value.qdict[i].value, obj)) {
-                return 0;
+                return false;
             }
         }
 
-        return 1;
+        return true;
     }
     case QTYPE_QLIST: {
         QListCompareHelper helper;
 
         helper.index = 0;
         helper.objs = lhs->value.qlist;
-        helper.result = 1;
+        helper.result = true;
 
         qlist_iter(qobject_to_qlist(rhs), compare_helper, &helper);
 
@@ -85,5 +85,5 @@ int qlit_equal_qobject(QLitObject *lhs, QObject *rhs)
         break;
     }
 
-    return 0;
+    return false;
 }
diff --git a/tests/check-qjson.c b/tests/check-qjson.c
index e5ca273cbc..59227934ce 100644
--- a/tests/check-qjson.c
+++ b/tests/check-qjson.c
@@ -1094,13 +1094,13 @@ static void simple_dict(void)
         QString *str;
 
         obj = qobject_from_json(test_cases[i].encoded, &error_abort);
-        g_assert(qlit_equal_qobject(&test_cases[i].decoded, obj) == 1);
+        g_assert(qlit_equal_qobject(&test_cases[i].decoded, obj));
 
         str = qobject_to_json(obj);
         qobject_decref(obj);
 
         obj = qobject_from_json(qstring_get_str(str), &error_abort);
-        g_assert(qlit_equal_qobject(&test_cases[i].decoded, obj) == 1);
+        g_assert(qlit_equal_qobject(&test_cases[i].decoded, obj));
         qobject_decref(obj);
         QDECREF(str);
     }
@@ -1203,13 +1203,13 @@ static void simple_list(void)
         QString *str;
 
         obj = qobject_from_json(test_cases[i].encoded, &error_abort);
-        g_assert(qlit_equal_qobject(&test_cases[i].decoded, obj) == 1);
+        g_assert(qlit_equal_qobject(&test_cases[i].decoded, obj));
 
         str = qobject_to_json(obj);
         qobject_decref(obj);
 
         obj = qobject_from_json(qstring_get_str(str), &error_abort);
-        g_assert(qlit_equal_qobject(&test_cases[i].decoded, obj) == 1);
+        g_assert(qlit_equal_qobject(&test_cases[i].decoded, obj));
         qobject_decref(obj);
         QDECREF(str);
     }
@@ -1265,13 +1265,13 @@ static void simple_whitespace(void)
         QString *str;
 
         obj = qobject_from_json(test_cases[i].encoded, &error_abort);
-        g_assert(qlit_equal_qobject(&test_cases[i].decoded, obj) == 1);
+        g_assert(qlit_equal_qobject(&test_cases[i].decoded, obj));
 
         str = qobject_to_json(obj);
         qobject_decref(obj);
 
         obj = qobject_from_json(qstring_get_str(str), &error_abort);
-        g_assert(qlit_equal_qobject(&test_cases[i].decoded, obj) == 1);
+        g_assert(qlit_equal_qobject(&test_cases[i].decoded, obj));
 
         qobject_decref(obj);
         QDECREF(str);
@@ -1295,7 +1295,7 @@ static void simple_varargs(void)
     g_assert(embedded_obj != NULL);
 
     obj = qobject_from_jsonf("[%d, 2, %p]", 1, embedded_obj);
-    g_assert(qlit_equal_qobject(&decoded, obj) == 1);
+    g_assert(qlit_equal_qobject(&decoded, obj));
 
     qobject_decref(obj);
 }
-- 
2.14.1.146.gd35faa819

[Qemu-devel] [PATCH v2 07/14] qlit: make qlit_equal_qobject() take const arguments

[Marc-André Lureau]

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
---
 include/qapi/qmp/qlit.h | 2 +-
 qobject/qlit.c          | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/include/qapi/qmp/qlit.h b/include/qapi/qmp/qlit.h
index 35aabbdc9f..fc1a2d845d 100644
--- a/include/qapi/qmp/qlit.h
+++ b/include/qapi/qmp/qlit.h
@@ -44,6 +44,6 @@ struct QLitDictEntry {
 #define QLIT_QLIST(val) \
     { .type = QTYPE_QLIST, .value.qlist = (val) }
 
-bool qlit_equal_qobject(QLitObject *lhs, QObject *rhs);
+bool qlit_equal_qobject(const QLitObject *lhs, const QObject *rhs);
 
 #endif /* QLIT_H */
diff --git a/qobject/qlit.c b/qobject/qlit.c
index a2975bef3f..ae2787ef35 100644
--- a/qobject/qlit.c
+++ b/qobject/qlit.c
@@ -41,7 +41,7 @@ static void compare_helper(QObject *obj, void *opaque)
         qlit_equal_qobject(&helper->objs[helper->index++], obj);
 }
 
-bool qlit_equal_qobject(QLitObject *lhs, QObject *rhs)
+bool qlit_equal_qobject(const QLitObject *lhs, const QObject *rhs)
 {
     int64_t val;
 
-- 
2.14.1.146.gd35faa819

[Qemu-devel] [PATCH v2 08/14] qlit: add QLIT_QNULL and QLIT_BOOL

[Marc-André Lureau]

As they are going to be used in the following patches.

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
---
 include/qapi/qmp/qlit.h | 5 +++++
 qobject/qlit.c          | 4 ++++
 2 files changed, 9 insertions(+)

diff --git a/include/qapi/qmp/qlit.h b/include/qapi/qmp/qlit.h
index fc1a2d845d..b18406bce9 100644
--- a/include/qapi/qmp/qlit.h
+++ b/include/qapi/qmp/qlit.h
@@ -23,6 +23,7 @@ typedef struct QLitObject QLitObject;
 struct QLitObject {
     int type;
     union {
+        bool qbool;
         int64_t qnum;
         const char *qstr;
         QLitDictEntry *qdict;
@@ -35,6 +36,10 @@ struct QLitDictEntry {
     QLitObject value;
 };
 
+#define QLIT_QNULL \
+    { .type = QTYPE_QNULL }
+#define QLIT_QBOOL(val) \
+    { .type = QTYPE_QBOOL, .value.qbool = (val) }
 #define QLIT_QNUM(val) \
     { .type = QTYPE_QNUM, .value.qnum = (val) }
 #define QLIT_QSTR(val) \
diff --git a/qobject/qlit.c b/qobject/qlit.c
index ae2787ef35..07ad6b05e8 100644
--- a/qobject/qlit.c
+++ b/qobject/qlit.c
@@ -50,6 +50,8 @@ bool qlit_equal_qobject(const QLitObject *lhs, const QObject *rhs)
     }
 
     switch (lhs->type) {
+    case QTYPE_QBOOL:
+        return lhs->value.qbool == qbool_get_bool(qobject_to_qbool(rhs));
     case QTYPE_QNUM:
         g_assert(qnum_get_try_int(qobject_to_qnum(rhs), &val));
         return lhs->value.qnum == val;
@@ -81,6 +83,8 @@ bool qlit_equal_qobject(const QLitObject *lhs, const QObject *rhs)
 
         return helper.result;
     }
+    case QTYPE_QNULL:
+        return true;
     default:
         break;
     }
-- 
2.14.1.146.gd35faa819

[Qemu-devel] [PATCH v2 09/14] qlit: Replace open-coded qnum_get_int() by call

[Marc-André Lureau]

Bonus: rids us of a side effect in an assertion.

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
---
 qobject/qlit.c | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/qobject/qlit.c b/qobject/qlit.c
index 07ad6b05e8..b1d9146220 100644
--- a/qobject/qlit.c
+++ b/qobject/qlit.c
@@ -43,8 +43,6 @@ static void compare_helper(QObject *obj, void *opaque)
 
 bool qlit_equal_qobject(const QLitObject *lhs, const QObject *rhs)
 {
-    int64_t val;
-
     if (!rhs || lhs->type != qobject_type(rhs)) {
         return false;
     }
@@ -53,8 +51,7 @@ bool qlit_equal_qobject(const QLitObject *lhs, const QObject *rhs)
     case QTYPE_QBOOL:
         return lhs->value.qbool == qbool_get_bool(qobject_to_qbool(rhs));
     case QTYPE_QNUM:
-        g_assert(qnum_get_try_int(qobject_to_qnum(rhs), &val));
-        return lhs->value.qnum == val;
+        return lhs->value.qnum ==  qnum_get_int(qobject_to_qnum(rhs));
     case QTYPE_QSTRING:
         return (strcmp(lhs->value.qstr,
                        qstring_get_str(qobject_to_qstring(rhs))) == 0);
-- 
2.14.1.146.gd35faa819

[Qemu-devel] [PATCH v2 10/14] tests: add qlit tests

[Marc-André Lureau]

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
 tests/check-qlit.c     | 64 ++++++++++++++++++++++++++++++++++++++++++++++++++
 tests/Makefile.include |  5 +++-
 2 files changed, 68 insertions(+), 1 deletion(-)
 create mode 100644 tests/check-qlit.c

diff --git a/tests/check-qlit.c b/tests/check-qlit.c
new file mode 100644
index 0000000000..47fde92a46
--- /dev/null
+++ b/tests/check-qlit.c
@@ -0,0 +1,64 @@
+/*
+ * QLit unit-tests.
+ *
+ * Copyright (C) 2017 Red Hat Inc.
+ *
+ * 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/qbool.h"
+#include "qapi/qmp/qdict.h"
+#include "qapi/qmp/qlit.h"
+#include "qapi/qmp/qnum.h"
+#include "qapi/qmp/qstring.h"
+
+static QLitObject qlit = QLIT_QDICT(((QLitDictEntry[]) {
+    { "foo", QLIT_QNUM(42) },
+    { "bar", QLIT_QSTR("hello world") },
+    { "baz", QLIT_QNULL },
+    { "bee", QLIT_QLIST(((QLitObject[]) {
+        QLIT_QNUM(43),
+        QLIT_QNUM(44),
+        QLIT_QBOOL(true),
+        { },
+    }))},
+    { },
+}));
+
+static QObject *make_qobject(void)
+{
+    QDict *qdict = qdict_new();
+    QList *list = qlist_new();
+
+    qdict_put_int(qdict, "foo", 42);
+    qdict_put_str(qdict, "bar", "hello world");
+    qdict_put_null(qdict, "baz");
+
+    qlist_append_int(list, 43);
+    qlist_append_int(list, 44);
+    qlist_append_bool(list, true);
+    qdict_put(qdict, "bee", list);
+
+    return QOBJECT(qdict);
+}
+
+static void qlit_equal_qobject_test(void)
+{
+    QObject *qobj = make_qobject();
+
+    g_assert(qlit_equal_qobject(&qlit, qobj));
+
+    qobject_decref(qobj);
+}
+
+int main(int argc, char **argv)
+{
+    g_test_init(&argc, &argv, NULL);
+
+    g_test_add_func("/qlit/equal_qobject", qlit_equal_qobject_test);
+
+    return g_test_run();
+}
diff --git a/tests/Makefile.include b/tests/Makefile.include
index 37c1bed683..3653c7b40a 100644
--- a/tests/Makefile.include
+++ b/tests/Makefile.include
@@ -43,6 +43,8 @@ check-unit-y += tests/check-qnull$(EXESUF)
 gcov-files-check-qnull-y = qobject/qnull.c
 check-unit-y += tests/check-qjson$(EXESUF)
 gcov-files-check-qjson-y = qobject/qjson.c
+check-unit-y += tests/check-qlit$(EXESUF)
+gcov-files-check-qlit-y = qobject/qlit.c
 check-unit-y += tests/test-qobject-output-visitor$(EXESUF)
 gcov-files-test-qobject-output-visitor-y = qapi/qobject-output-visitor.c
 check-unit-y += tests/test-clone-visitor$(EXESUF)
@@ -535,7 +537,7 @@ GENERATED_FILES += tests/test-qapi-types.h tests/test-qapi-visit.h \
 
 test-obj-y = tests/check-qnum.o tests/check-qstring.o tests/check-qdict.o \
 	tests/check-qlist.o tests/check-qnull.o \
-	tests/check-qjson.o \
+	tests/check-qjson.o tests/check-qlit.o \
 	tests/test-coroutine.o tests/test-string-output-visitor.o \
 	tests/test-string-input-visitor.o tests/test-qobject-output-visitor.o \
 	tests/test-clone-visitor.o \
@@ -569,6 +571,7 @@ tests/check-qdict$(EXESUF): tests/check-qdict.o $(test-util-obj-y)
 tests/check-qlist$(EXESUF): tests/check-qlist.o $(test-util-obj-y)
 tests/check-qnull$(EXESUF): tests/check-qnull.o $(test-util-obj-y)
 tests/check-qjson$(EXESUF): tests/check-qjson.o $(test-util-obj-y)
+tests/check-qlit$(EXESUF): tests/check-qlit.o $(test-util-obj-y)
 tests/check-qom-interface$(EXESUF): tests/check-qom-interface.o $(test-qom-obj-y)
 tests/check-qom-proplist$(EXESUF): tests/check-qom-proplist.o $(test-qom-obj-y)
 
-- 
2.14.1.146.gd35faa819

Re: [Qemu-devel] [PATCH v2 10/14] tests: add qlit tests

[Markus Armbruster]

Marc-André Lureau <marcandre.lureau@redhat.com> writes:

> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> ---
>  tests/check-qlit.c     | 64 ++++++++++++++++++++++++++++++++++++++++++++++++++
>  tests/Makefile.include |  5 +++-
>  2 files changed, 68 insertions(+), 1 deletion(-)
>  create mode 100644 tests/check-qlit.c
>
> diff --git a/tests/check-qlit.c b/tests/check-qlit.c
> new file mode 100644
> index 0000000000..47fde92a46
> --- /dev/null
> +++ b/tests/check-qlit.c
> @@ -0,0 +1,64 @@
> +/*
> + * QLit unit-tests.
> + *
> + * Copyright (C) 2017 Red Hat Inc.
> + *
> + * 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.

Please use GPLv2+ unless you have a specific reason for something else.
In that case, explain your reason in the commit message.

May I change the copyright notice to GPLv2+ for you?

> + */
> +
> +#include "qemu/osdep.h"
> +
> +#include "qapi/qmp/qbool.h"
> +#include "qapi/qmp/qdict.h"
> +#include "qapi/qmp/qlit.h"
> +#include "qapi/qmp/qnum.h"
> +#include "qapi/qmp/qstring.h"
> +
> +static QLitObject qlit = QLIT_QDICT(((QLitDictEntry[]) {
> +    { "foo", QLIT_QNUM(42) },
> +    { "bar", QLIT_QSTR("hello world") },
> +    { "baz", QLIT_QNULL },
> +    { "bee", QLIT_QLIST(((QLitObject[]) {
> +        QLIT_QNUM(43),
> +        QLIT_QNUM(44),
> +        QLIT_QBOOL(true),
> +        { },
> +    }))},
> +    { },
> +}));
> +
> +static QObject *make_qobject(void)
> +{
> +    QDict *qdict = qdict_new();
> +    QList *list = qlist_new();
> +
> +    qdict_put_int(qdict, "foo", 42);
> +    qdict_put_str(qdict, "bar", "hello world");
> +    qdict_put_null(qdict, "baz");
> +
> +    qlist_append_int(list, 43);
> +    qlist_append_int(list, 44);
> +    qlist_append_bool(list, true);
> +    qdict_put(qdict, "bee", list);
> +
> +    return QOBJECT(qdict);
> +}
> +
> +static void qlit_equal_qobject_test(void)
> +{
> +    QObject *qobj = make_qobject();
> +
> +    g_assert(qlit_equal_qobject(&qlit, qobj));
> +
> +    qobject_decref(qobj);
> +}
> +
> +int main(int argc, char **argv)
> +{
> +    g_test_init(&argc, &argv, NULL);
> +
> +    g_test_add_func("/qlit/equal_qobject", qlit_equal_qobject_test);
> +
> +    return g_test_run();
> +}

We also need negative tests.  Could be done as follow-up patch.

> diff --git a/tests/Makefile.include b/tests/Makefile.include
> index 37c1bed683..3653c7b40a 100644
> --- a/tests/Makefile.include
> +++ b/tests/Makefile.include
> @@ -43,6 +43,8 @@ check-unit-y += tests/check-qnull$(EXESUF)
>  gcov-files-check-qnull-y = qobject/qnull.c
>  check-unit-y += tests/check-qjson$(EXESUF)
>  gcov-files-check-qjson-y = qobject/qjson.c
> +check-unit-y += tests/check-qlit$(EXESUF)
> +gcov-files-check-qlit-y = qobject/qlit.c
>  check-unit-y += tests/test-qobject-output-visitor$(EXESUF)
>  gcov-files-test-qobject-output-visitor-y = qapi/qobject-output-visitor.c
>  check-unit-y += tests/test-clone-visitor$(EXESUF)
> @@ -535,7 +537,7 @@ GENERATED_FILES += tests/test-qapi-types.h tests/test-qapi-visit.h \
>  
>  test-obj-y = tests/check-qnum.o tests/check-qstring.o tests/check-qdict.o \
>  	tests/check-qlist.o tests/check-qnull.o \
> -	tests/check-qjson.o \
> +	tests/check-qjson.o tests/check-qlit.o \
>  	tests/test-coroutine.o tests/test-string-output-visitor.o \
>  	tests/test-string-input-visitor.o tests/test-qobject-output-visitor.o \
>  	tests/test-clone-visitor.o \
> @@ -569,6 +571,7 @@ tests/check-qdict$(EXESUF): tests/check-qdict.o $(test-util-obj-y)
>  tests/check-qlist$(EXESUF): tests/check-qlist.o $(test-util-obj-y)
>  tests/check-qnull$(EXESUF): tests/check-qnull.o $(test-util-obj-y)
>  tests/check-qjson$(EXESUF): tests/check-qjson.o $(test-util-obj-y)
> +tests/check-qlit$(EXESUF): tests/check-qlit.o $(test-util-obj-y)
>  tests/check-qom-interface$(EXESUF): tests/check-qom-interface.o $(test-qom-obj-y)
>  tests/check-qom-proplist$(EXESUF): tests/check-qom-proplist.o $(test-qom-obj-y)

Re: [Qemu-devel] [PATCH v2 10/14] tests: add qlit tests

[Marc-André Lureau]

Hi

----- Original Message -----
> Marc-André Lureau <marcandre.lureau@redhat.com> writes:
> 
> > Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> > ---
> >  tests/check-qlit.c     | 64
> >  ++++++++++++++++++++++++++++++++++++++++++++++++++
> >  tests/Makefile.include |  5 +++-
> >  2 files changed, 68 insertions(+), 1 deletion(-)
> >  create mode 100644 tests/check-qlit.c
> >
> > diff --git a/tests/check-qlit.c b/tests/check-qlit.c
> > new file mode 100644
> > index 0000000000..47fde92a46
> > --- /dev/null
> > +++ b/tests/check-qlit.c
> > @@ -0,0 +1,64 @@
> > +/*
> > + * QLit unit-tests.
> > + *
> > + * Copyright (C) 2017 Red Hat Inc.
> > + *
> > + * 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.
> 
> Please use GPLv2+ unless you have a specific reason for something else.
> In that case, explain your reason in the commit message.
> 
> May I change the copyright notice to GPLv2+ for you?

I copied that header from some other files. Sure, feel free (although I like LGPL better in general, so we can easily move the code to a shared library)

> 
> > + */
> > +
> > +#include "qemu/osdep.h"
> > +
> > +#include "qapi/qmp/qbool.h"
> > +#include "qapi/qmp/qdict.h"
> > +#include "qapi/qmp/qlit.h"
> > +#include "qapi/qmp/qnum.h"
> > +#include "qapi/qmp/qstring.h"
> > +
> > +static QLitObject qlit = QLIT_QDICT(((QLitDictEntry[]) {
> > +    { "foo", QLIT_QNUM(42) },
> > +    { "bar", QLIT_QSTR("hello world") },
> > +    { "baz", QLIT_QNULL },
> > +    { "bee", QLIT_QLIST(((QLitObject[]) {
> > +        QLIT_QNUM(43),
> > +        QLIT_QNUM(44),
> > +        QLIT_QBOOL(true),
> > +        { },
> > +    }))},
> > +    { },
> > +}));
> > +
> > +static QObject *make_qobject(void)
> > +{
> > +    QDict *qdict = qdict_new();
> > +    QList *list = qlist_new();
> > +
> > +    qdict_put_int(qdict, "foo", 42);
> > +    qdict_put_str(qdict, "bar", "hello world");
> > +    qdict_put_null(qdict, "baz");
> > +
> > +    qlist_append_int(list, 43);
> > +    qlist_append_int(list, 44);
> > +    qlist_append_bool(list, true);
> > +    qdict_put(qdict, "bee", list);
> > +
> > +    return QOBJECT(qdict);
> > +}
> > +
> > +static void qlit_equal_qobject_test(void)
> > +{
> > +    QObject *qobj = make_qobject();
> > +
> > +    g_assert(qlit_equal_qobject(&qlit, qobj));
> > +
> > +    qobject_decref(qobj);
> > +}
> > +
> > +int main(int argc, char **argv)
> > +{
> > +    g_test_init(&argc, &argv, NULL);
> > +
> > +    g_test_add_func("/qlit/equal_qobject", qlit_equal_qobject_test);
> > +
> > +    return g_test_run();
> > +}
> 
> We also need negative tests.  Could be done as follow-up patch.

There are a few in following patches iirc, more is always welcome.

Thanks

> 
> > diff --git a/tests/Makefile.include b/tests/Makefile.include
> > index 37c1bed683..3653c7b40a 100644
> > --- a/tests/Makefile.include
> > +++ b/tests/Makefile.include
> > @@ -43,6 +43,8 @@ check-unit-y += tests/check-qnull$(EXESUF)
> >  gcov-files-check-qnull-y = qobject/qnull.c
> >  check-unit-y += tests/check-qjson$(EXESUF)
> >  gcov-files-check-qjson-y = qobject/qjson.c
> > +check-unit-y += tests/check-qlit$(EXESUF)
> > +gcov-files-check-qlit-y = qobject/qlit.c
> >  check-unit-y += tests/test-qobject-output-visitor$(EXESUF)
> >  gcov-files-test-qobject-output-visitor-y = qapi/qobject-output-visitor.c
> >  check-unit-y += tests/test-clone-visitor$(EXESUF)
> > @@ -535,7 +537,7 @@ GENERATED_FILES += tests/test-qapi-types.h
> > tests/test-qapi-visit.h \
> >  
> >  test-obj-y = tests/check-qnum.o tests/check-qstring.o tests/check-qdict.o
> >  \
> >  	tests/check-qlist.o tests/check-qnull.o \
> > -	tests/check-qjson.o \
> > +	tests/check-qjson.o tests/check-qlit.o \
> >  	tests/test-coroutine.o tests/test-string-output-visitor.o \
> >  	tests/test-string-input-visitor.o tests/test-qobject-output-visitor.o \
> >  	tests/test-clone-visitor.o \
> > @@ -569,6 +571,7 @@ tests/check-qdict$(EXESUF): tests/check-qdict.o
> > $(test-util-obj-y)
> >  tests/check-qlist$(EXESUF): tests/check-qlist.o $(test-util-obj-y)
> >  tests/check-qnull$(EXESUF): tests/check-qnull.o $(test-util-obj-y)
> >  tests/check-qjson$(EXESUF): tests/check-qjson.o $(test-util-obj-y)
> > +tests/check-qlit$(EXESUF): tests/check-qlit.o $(test-util-obj-y)
> >  tests/check-qom-interface$(EXESUF): tests/check-qom-interface.o
> >  $(test-qom-obj-y)
> >  tests/check-qom-proplist$(EXESUF): tests/check-qom-proplist.o
> >  $(test-qom-obj-y)
> 

[Qemu-devel] [PATCH v2 11/14] qlit: improve QLit dict vs qdict comparison

[Marc-André Lureau]

Verify that all qdict values are covered.

(a previous iteration of this patch created a copy of qdict to remove
all checked elements, verifying no duplicates exist in the literal
qdict, however this is a programming error, and a size comparison
should be enough)

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
 qobject/qlit.c     | 37 +++++++++++++++++++++++--------------
 tests/check-qlit.c |  7 +++++++
 2 files changed, 30 insertions(+), 14 deletions(-)

diff --git a/qobject/qlit.c b/qobject/qlit.c
index b1d9146220..dc0015f76c 100644
--- a/qobject/qlit.c
+++ b/qobject/qlit.c
@@ -41,6 +41,27 @@ static void compare_helper(QObject *obj, void *opaque)
         qlit_equal_qobject(&helper->objs[helper->index++], obj);
 }
 
+static bool qlit_equal_qdict(const QLitObject *lhs, const QDict *qdict)
+{
+    int i;
+
+    for (i = 0; lhs->value.qdict[i].key; i++) {
+        QObject *obj = qdict_get(qdict, lhs->value.qdict[i].key);
+
+        if (!qlit_equal_qobject(&lhs->value.qdict[i].value, obj)) {
+            return false;
+        }
+    }
+
+    /* Note: the literal qdict must not contain duplicates, this is
+     * considered a programming error and it isn't checked here. */
+    if (qdict_size(qdict) != i) {
+        return false;
+    }
+
+    return true;
+}
+
 bool qlit_equal_qobject(const QLitObject *lhs, const QObject *rhs)
 {
     if (!rhs || lhs->type != qobject_type(rhs)) {
@@ -55,20 +76,8 @@ bool qlit_equal_qobject(const QLitObject *lhs, const QObject *rhs)
     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 (!qlit_equal_qobject(&lhs->value.qdict[i].value, obj)) {
-                return false;
-            }
-        }
-
-        return true;
-    }
+    case QTYPE_QDICT:
+        return qlit_equal_qdict(lhs, qobject_to_qdict(rhs));
     case QTYPE_QLIST: {
         QListCompareHelper helper;
 
diff --git a/tests/check-qlit.c b/tests/check-qlit.c
index 47fde92a46..5d9558dfd9 100644
--- a/tests/check-qlit.c
+++ b/tests/check-qlit.c
@@ -28,6 +28,11 @@ static QLitObject qlit = QLIT_QDICT(((QLitDictEntry[]) {
     { },
 }));
 
+static QLitObject qlit_foo = QLIT_QDICT(((QLitDictEntry[]) {
+    { "foo", QLIT_QNUM(42) },
+    { },
+}));
+
 static QObject *make_qobject(void)
 {
     QDict *qdict = qdict_new();
@@ -51,6 +56,8 @@ static void qlit_equal_qobject_test(void)
 
     g_assert(qlit_equal_qobject(&qlit, qobj));
 
+    g_assert(!qlit_equal_qobject(&qlit_foo, qobj));
+
     qobject_decref(qobj);
 }
 
-- 
2.14.1.146.gd35faa819

Re: [Qemu-devel] [PATCH v2 11/14] qlit: improve QLit dict vs qdict comparison

[Markus Armbruster]

Marc-André Lureau <marcandre.lureau@redhat.com> writes:

> Verify that all qdict values are covered.
>
> (a previous iteration of this patch created a copy of qdict to remove
> all checked elements, verifying no duplicates exist in the literal
> qdict, however this is a programming error, and a size comparison
> should be enough)

The parenthesis confused me, because I wasn't sure about the exact
meaning of "a previous iteration of this patch".  I tried to find a
prior commit, but it looks like you're really talking about v1.
Confused me, because I don't expect commit messages discussing previous
iterations.

What about:

    qlit: Tighten QLit dict vs qdict comparison

    We check that all members of the QLit dictionary are also in the
    QDict.  We neglect to check the other direction.

    Comparing the number of members suffices, because QDict can't
    contain duplicate members, and putting duplicates in a QLit is a
    programming error.

> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> ---
>  qobject/qlit.c     | 37 +++++++++++++++++++++++--------------
>  tests/check-qlit.c |  7 +++++++
>  2 files changed, 30 insertions(+), 14 deletions(-)
>
> diff --git a/qobject/qlit.c b/qobject/qlit.c
> index b1d9146220..dc0015f76c 100644
> --- a/qobject/qlit.c
> +++ b/qobject/qlit.c
> @@ -41,6 +41,27 @@ static void compare_helper(QObject *obj, void *opaque)
>          qlit_equal_qobject(&helper->objs[helper->index++], obj);
>  }
>  
> +static bool qlit_equal_qdict(const QLitObject *lhs, const QDict *qdict)
> +{
> +    int i;
> +
> +    for (i = 0; lhs->value.qdict[i].key; i++) {
> +        QObject *obj = qdict_get(qdict, lhs->value.qdict[i].key);
> +
> +        if (!qlit_equal_qobject(&lhs->value.qdict[i].value, obj)) {
> +            return false;
> +        }
> +    }
> +
> +    /* Note: the literal qdict must not contain duplicates, this is
> +     * considered a programming error and it isn't checked here. */
> +    if (qdict_size(qdict) != i) {
> +        return false;
> +    }
> +
> +    return true;
> +}
> +
>  bool qlit_equal_qobject(const QLitObject *lhs, const QObject *rhs)
>  {
>      if (!rhs || lhs->type != qobject_type(rhs)) {
> @@ -55,20 +76,8 @@ bool qlit_equal_qobject(const QLitObject *lhs, const QObject *rhs)
>      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 (!qlit_equal_qobject(&lhs->value.qdict[i].value, obj)) {
> -                return false;
> -            }
> -        }
> -
> -        return true;
> -    }
> +    case QTYPE_QDICT:
> +        return qlit_equal_qdict(lhs, qobject_to_qdict(rhs));
>      case QTYPE_QLIST: {
>          QListCompareHelper helper;
>  
> diff --git a/tests/check-qlit.c b/tests/check-qlit.c
> index 47fde92a46..5d9558dfd9 100644
> --- a/tests/check-qlit.c
> +++ b/tests/check-qlit.c
> @@ -28,6 +28,11 @@ static QLitObject qlit = QLIT_QDICT(((QLitDictEntry[]) {
>      { },
>  }));
>  
> +static QLitObject qlit_foo = QLIT_QDICT(((QLitDictEntry[]) {
> +    { "foo", QLIT_QNUM(42) },
> +    { },
> +}));
> +
>  static QObject *make_qobject(void)
>  {
>      QDict *qdict = qdict_new();
> @@ -51,6 +56,8 @@ static void qlit_equal_qobject_test(void)
>  
>      g_assert(qlit_equal_qobject(&qlit, qobj));
>  
> +    g_assert(!qlit_equal_qobject(&qlit_foo, qobj));
> +
>      qobject_decref(qobj);
>  }

Ah, you do have negative test cases, just not in the patch creating the
test.

When you leave gaps in that will be filled in later patches, I recomend
pointing out the gaps in the commit message, with a promise they'll be
filled shortly.  Your reviewers will thank you.

With am improved commit message,
Reviewed-by: Markus Armbruster <armbru@redhat.com>

Re: [Qemu-devel] [PATCH v2 11/14] qlit: improve QLit dict vs qdict comparison

[Marc-André Lureau]

Hi

----- Original Message -----
> Marc-André Lureau <marcandre.lureau@redhat.com> writes:
> 
> > Verify that all qdict values are covered.
> >
> > (a previous iteration of this patch created a copy of qdict to remove
> > all checked elements, verifying no duplicates exist in the literal
> > qdict, however this is a programming error, and a size comparison
> > should be enough)
> 
> The parenthesis confused me, because I wasn't sure about the exact
> meaning of "a previous iteration of this patch".  I tried to find a
> prior commit, but it looks like you're really talking about v1.
> Confused me, because I don't expect commit messages discussing previous
> iterations.
> 
> What about:
> 
>     qlit: Tighten QLit dict vs qdict comparison
> 
>     We check that all members of the QLit dictionary are also in the
>     QDict.  We neglect to check the other direction.
> 
>     Comparing the number of members suffices, because QDict can't
>     contain duplicate members, and putting duplicates in a QLit is a
>     programming error.

Looks good to me

> 
> > Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> > ---
> >  qobject/qlit.c     | 37 +++++++++++++++++++++++--------------
> >  tests/check-qlit.c |  7 +++++++
> >  2 files changed, 30 insertions(+), 14 deletions(-)
> >
> > diff --git a/qobject/qlit.c b/qobject/qlit.c
> > index b1d9146220..dc0015f76c 100644
> > --- a/qobject/qlit.c
> > +++ b/qobject/qlit.c
> > @@ -41,6 +41,27 @@ static void compare_helper(QObject *obj, void *opaque)
> >          qlit_equal_qobject(&helper->objs[helper->index++], obj);
> >  }
> >  
> > +static bool qlit_equal_qdict(const QLitObject *lhs, const QDict *qdict)
> > +{
> > +    int i;
> > +
> > +    for (i = 0; lhs->value.qdict[i].key; i++) {
> > +        QObject *obj = qdict_get(qdict, lhs->value.qdict[i].key);
> > +
> > +        if (!qlit_equal_qobject(&lhs->value.qdict[i].value, obj)) {
> > +            return false;
> > +        }
> > +    }
> > +
> > +    /* Note: the literal qdict must not contain duplicates, this is
> > +     * considered a programming error and it isn't checked here. */
> > +    if (qdict_size(qdict) != i) {
> > +        return false;
> > +    }
> > +
> > +    return true;
> > +}
> > +
> >  bool qlit_equal_qobject(const QLitObject *lhs, const QObject *rhs)
> >  {
> >      if (!rhs || lhs->type != qobject_type(rhs)) {
> > @@ -55,20 +76,8 @@ bool qlit_equal_qobject(const QLitObject *lhs, const
> > QObject *rhs)
> >      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 (!qlit_equal_qobject(&lhs->value.qdict[i].value, obj)) {
> > -                return false;
> > -            }
> > -        }
> > -
> > -        return true;
> > -    }
> > +    case QTYPE_QDICT:
> > +        return qlit_equal_qdict(lhs, qobject_to_qdict(rhs));
> >      case QTYPE_QLIST: {
> >          QListCompareHelper helper;
> >  
> > diff --git a/tests/check-qlit.c b/tests/check-qlit.c
> > index 47fde92a46..5d9558dfd9 100644
> > --- a/tests/check-qlit.c
> > +++ b/tests/check-qlit.c
> > @@ -28,6 +28,11 @@ static QLitObject qlit = QLIT_QDICT(((QLitDictEntry[]) {
> >      { },
> >  }));
> >  
> > +static QLitObject qlit_foo = QLIT_QDICT(((QLitDictEntry[]) {
> > +    { "foo", QLIT_QNUM(42) },
> > +    { },
> > +}));
> > +
> >  static QObject *make_qobject(void)
> >  {
> >      QDict *qdict = qdict_new();
> > @@ -51,6 +56,8 @@ static void qlit_equal_qobject_test(void)
> >  
> >      g_assert(qlit_equal_qobject(&qlit, qobj));
> >  
> > +    g_assert(!qlit_equal_qobject(&qlit_foo, qobj));
> > +
> >      qobject_decref(qobj);
> >  }
> 
> Ah, you do have negative test cases, just not in the patch creating the
> test.
> 
> When you leave gaps in that will be filled in later patches, I recomend
> pointing out the gaps in the commit message, with a promise they'll be
> filled shortly.  Your reviewers will thank you.
> 
> With am improved commit message,
> Reviewed-by: Markus Armbruster <armbru@redhat.com>
> 

Thanks

[Qemu-devel] [PATCH v2 12/14] qlit: improve QLit list vs qlist comparison

[Marc-André Lureau]

Use QLIST_FOREACH_ENTRY() to simplify the code and break earlier.

Check that the QLit list has the same size as the qlist, this should
ensure that we have an exact match when iterating over qlist for
comparing the elements.

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
 qobject/qlit.c     | 53 +++++++++++++++++++----------------------------------
 tests/check-qlit.c |  3 +++
 2 files changed, 22 insertions(+), 34 deletions(-)

diff --git a/qobject/qlit.c b/qobject/qlit.c
index dc0015f76c..3c4882c784 100644
--- a/qobject/qlit.c
+++ b/qobject/qlit.c
@@ -18,29 +18,6 @@
 #include "qapi/qmp/qlit.h"
 #include "qapi/qmp/types.h"
 
-typedef struct QListCompareHelper {
-    int index;
-    QLitObject *objs;
-    bool result;
-} QListCompareHelper;
-
-static void compare_helper(QObject *obj, void *opaque)
-{
-    QListCompareHelper *helper = opaque;
-
-    if (!helper->result) {
-        return;
-    }
-
-    if (helper->objs[helper->index].type == QTYPE_NONE) {
-        helper->result = false;
-        return;
-    }
-
-    helper->result =
-        qlit_equal_qobject(&helper->objs[helper->index++], obj);
-}
-
 static bool qlit_equal_qdict(const QLitObject *lhs, const QDict *qdict)
 {
     int i;
@@ -62,6 +39,23 @@ static bool qlit_equal_qdict(const QLitObject *lhs, const QDict *qdict)
     return true;
 }
 
+static bool qlit_equal_qlist(const QLitObject *lhs, const QList *qlist)
+{
+    QListEntry *e;
+    int i = 0;
+
+    QLIST_FOREACH_ENTRY(qlist, e) {
+        QObject *obj = qlist_entry_obj(e);
+
+        if (!qlit_equal_qobject(&lhs->value.qlist[i], obj)) {
+            return false;
+        }
+        i++;
+    }
+
+    return !e && lhs->value.qlist[i].type == QTYPE_NONE;
+}
+
 bool qlit_equal_qobject(const QLitObject *lhs, const QObject *rhs)
 {
     if (!rhs || lhs->type != qobject_type(rhs)) {
@@ -78,17 +72,8 @@ bool qlit_equal_qobject(const QLitObject *lhs, const QObject *rhs)
                        qstring_get_str(qobject_to_qstring(rhs))) == 0);
     case QTYPE_QDICT:
         return qlit_equal_qdict(lhs, qobject_to_qdict(rhs));
-    case QTYPE_QLIST: {
-        QListCompareHelper helper;
-
-        helper.index = 0;
-        helper.objs = lhs->value.qlist;
-        helper.result = true;
-
-        qlist_iter(qobject_to_qlist(rhs), compare_helper, &helper);
-
-        return helper.result;
-    }
+    case QTYPE_QLIST:
+        return qlit_equal_qlist(lhs, qobject_to_qlist(rhs));
     case QTYPE_QNULL:
         return true;
     default:
diff --git a/tests/check-qlit.c b/tests/check-qlit.c
index 5d9558dfd9..ae74bfcb83 100644
--- a/tests/check-qlit.c
+++ b/tests/check-qlit.c
@@ -58,6 +58,9 @@ static void qlit_equal_qobject_test(void)
 
     g_assert(!qlit_equal_qobject(&qlit_foo, qobj));
 
+    qdict_put(qobject_to_qdict(qobj), "bee", qlist_new());
+    g_assert(!qlit_equal_qobject(&qlit, qobj));
+
     qobject_decref(qobj);
 }
 
-- 
2.14.1.146.gd35faa819

Re: [Qemu-devel] [PATCH v2 12/14] qlit: improve QLit list vs qlist comparison

[Markus Armbruster]

Marc-André Lureau <marcandre.lureau@redhat.com> writes:

> Use QLIST_FOREACH_ENTRY() to simplify the code and break earlier.
>
> Check that the QLit list has the same size as the qlist, this should
> ensure that we have an exact match when iterating over qlist for
> comparing the elements.
>
> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>

Reviewed-by: Markus Armbruster <armbru@redhat.com>

Re: [Qemu-devel] [PATCH v2 12/14] qlit: improve QLit list vs qlist comparison

[Markus Armbruster]

Markus Armbruster <armbru@redhat.com> writes:

> Marc-André Lureau <marcandre.lureau@redhat.com> writes:
>
>> Use QLIST_FOREACH_ENTRY() to simplify the code and break earlier.
>>
>> Check that the QLit list has the same size as the qlist, this should
>> ensure that we have an exact match when iterating over qlist for
>> comparing the elements.
>>
>> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
>
> Reviewed-by: Markus Armbruster <armbru@redhat.com>

I'm updating the commit message so it continues to match the previous
commit's message:

    qlit: Tighten QLit list vs QList comparison

    We check that all members of the QLit list are also in the QList.  We
    neglect to check the other direction.  Fix that.

    While there, use QLIST_FOREACH_ENTRY() to simplify the code and break
    the loop on the first mismatch.

Hope that's okay with you.

Re: [Qemu-devel] [PATCH v2 12/14] qlit: improve QLit list vs qlist comparison

[Marc-André Lureau]

Hi

----- Original Message -----
> Markus Armbruster <armbru@redhat.com> writes:
> 
> > Marc-André Lureau <marcandre.lureau@redhat.com> writes:
> >
> >> Use QLIST_FOREACH_ENTRY() to simplify the code and break earlier.
> >>
> >> Check that the QLit list has the same size as the qlist, this should
> >> ensure that we have an exact match when iterating over qlist for
> >> comparing the elements.
> >>
> >> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> >
> > Reviewed-by: Markus Armbruster <armbru@redhat.com>
> 
> I'm updating the commit message so it continues to match the previous
> commit's message:
> 
>     qlit: Tighten QLit list vs QList comparison
> 
>     We check that all members of the QLit list are also in the QList.  We
>     neglect to check the other direction.  Fix that.
> 
>     While there, use QLIST_FOREACH_ENTRY() to simplify the code and break
>     the loop on the first mismatch.
> 
> Hope that's okay with you.

Works for me, thanks a lot.

[Qemu-devel] [PATCH v2 13/14] qlit: add qobject_form_qlit()

[Marc-André Lureau]

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
 include/qapi/qmp/qlit.h |  2 ++
 qobject/qlit.c          | 37 +++++++++++++++++++++++++++++++++++++
 tests/check-qlit.c      | 26 ++++++++++++++++++++++++++
 3 files changed, 65 insertions(+)

diff --git a/include/qapi/qmp/qlit.h b/include/qapi/qmp/qlit.h
index b18406bce9..56feb25e04 100644
--- a/include/qapi/qmp/qlit.h
+++ b/include/qapi/qmp/qlit.h
@@ -51,4 +51,6 @@ struct QLitDictEntry {
 
 bool qlit_equal_qobject(const QLitObject *lhs, const QObject *rhs);
 
+QObject *qobject_from_qlit(const QLitObject *qlit);
+
 #endif /* QLIT_H */
diff --git a/qobject/qlit.c b/qobject/qlit.c
index 3c4882c784..e44c09e9eb 100644
--- a/qobject/qlit.c
+++ b/qobject/qlit.c
@@ -82,3 +82,40 @@ bool qlit_equal_qobject(const QLitObject *lhs, const QObject *rhs)
 
     return false;
 }
+
+QObject *qobject_from_qlit(const QLitObject *qlit)
+{
+    int i;
+
+    switch (qlit->type) {
+    case QTYPE_QNULL:
+        return QOBJECT(qnull());
+    case QTYPE_QNUM:
+        return QOBJECT(qnum_from_int(qlit->value.qnum));
+    case QTYPE_QSTRING:
+        return QOBJECT(qstring_from_str(qlit->value.qstr));
+    case QTYPE_QDICT: {
+        QDict *qdict = qdict_new();
+        for (i = 0; qlit->value.qdict[i].value.type != QTYPE_NONE; i++) {
+            QLitDictEntry *e = &qlit->value.qdict[i];
+
+            qdict_put_obj(qdict, e->key, qobject_from_qlit(&e->value));
+        }
+        return QOBJECT(qdict);
+    }
+    case QTYPE_QLIST: {
+        QList *qlist = qlist_new();
+
+        for (i = 0; qlit->value.qlist[i].type != QTYPE_NONE; i++) {
+            qlist_append_obj(qlist, qobject_from_qlit(&qlit->value.qlist[i]));
+        }
+        return QOBJECT(qlist);
+    }
+    case QTYPE_QBOOL:
+        return QOBJECT(qbool_from_bool(qlit->value.qbool));
+    case QTYPE_NONE:
+        assert(0);
+    }
+
+    return NULL;
+}
diff --git a/tests/check-qlit.c b/tests/check-qlit.c
index ae74bfcb83..135a399c6d 100644
--- a/tests/check-qlit.c
+++ b/tests/check-qlit.c
@@ -64,11 +64,37 @@ static void qlit_equal_qobject_test(void)
     qobject_decref(qobj);
 }
 
+static void qobject_from_qlit_test(void)
+{
+    QObject *obj, *qobj = qobject_from_qlit(&qlit);
+    QDict *qdict;
+    QList *bee;
+
+    qdict = qobject_to_qdict(qobj);
+    g_assert_cmpint(qdict_get_int(qdict, "foo"), ==, 42);
+    g_assert_cmpstr(qdict_get_str(qdict, "bar"), ==, "hello world");
+    g_assert(qobject_type(qdict_get(qdict, "baz")) == QTYPE_QNULL);
+
+    bee = qdict_get_qlist(qdict, "bee");
+    obj = qlist_pop(bee);
+    g_assert_cmpint(qnum_get_int(qobject_to_qnum(obj)), ==, 43);
+    qobject_decref(obj);
+    obj = qlist_pop(bee);
+    g_assert_cmpint(qnum_get_int(qobject_to_qnum(obj)), ==, 44);
+    qobject_decref(obj);
+    obj = qlist_pop(bee);
+    g_assert(qbool_get_bool(qobject_to_qbool(obj)));
+    qobject_decref(obj);
+
+    qobject_decref(qobj);
+}
+
 int main(int argc, char **argv)
 {
     g_test_init(&argc, &argv, NULL);
 
     g_test_add_func("/qlit/equal_qobject", qlit_equal_qobject_test);
+    g_test_add_func("/qlit/qobject_from_qlit", qobject_from_qlit_test);
 
     return g_test_run();
 }
-- 
2.14.1.146.gd35faa819

Re: [Qemu-devel] [PATCH v2 13/14] qlit: add qobject_form_qlit()

[Eric Blake]

[-- Attachment #1: Type: text/plain, Size: 499 bytes --]

On 08/25/2017 05:59 AM, Marc-André Lureau wrote:

s/form/from/ in the subject

> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> ---
>  include/qapi/qmp/qlit.h |  2 ++
>  qobject/qlit.c          | 37 +++++++++++++++++++++++++++++++++++++
>  tests/check-qlit.c      | 26 ++++++++++++++++++++++++++
>  3 files changed, 65 insertions(+)
> 

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 619 bytes --]

Re: [Qemu-devel] [PATCH v2 13/14] qlit: add qobject_form_qlit()

[Markus Armbruster]

Eric already pointed out the typo in the title.

Marc-André Lureau <marcandre.lureau@redhat.com> writes:

> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> ---
>  include/qapi/qmp/qlit.h |  2 ++
>  qobject/qlit.c          | 37 +++++++++++++++++++++++++++++++++++++
>  tests/check-qlit.c      | 26 ++++++++++++++++++++++++++
>  3 files changed, 65 insertions(+)
>
> diff --git a/include/qapi/qmp/qlit.h b/include/qapi/qmp/qlit.h
> index b18406bce9..56feb25e04 100644
> --- a/include/qapi/qmp/qlit.h
> +++ b/include/qapi/qmp/qlit.h
> @@ -51,4 +51,6 @@ struct QLitDictEntry {
>  
>  bool qlit_equal_qobject(const QLitObject *lhs, const QObject *rhs);
>  
> +QObject *qobject_from_qlit(const QLitObject *qlit);
> +
>  #endif /* QLIT_H */
> diff --git a/qobject/qlit.c b/qobject/qlit.c
> index 3c4882c784..e44c09e9eb 100644
> --- a/qobject/qlit.c
> +++ b/qobject/qlit.c
> @@ -82,3 +82,40 @@ bool qlit_equal_qobject(const QLitObject *lhs, const QObject *rhs)
>  
>      return false;
>  }
> +
> +QObject *qobject_from_qlit(const QLitObject *qlit)
> +{
> +    int i;
> +
> +    switch (qlit->type) {
> +    case QTYPE_QNULL:
> +        return QOBJECT(qnull());
> +    case QTYPE_QNUM:
> +        return QOBJECT(qnum_from_int(qlit->value.qnum));

This line shows that QLitObject member value.qnum should really be named
value.qint.  Let's not worry about that now.

> +    case QTYPE_QSTRING:
> +        return QOBJECT(qstring_from_str(qlit->value.qstr));
> +    case QTYPE_QDICT: {
> +        QDict *qdict = qdict_new();

Blank line here.  Can touch up when I apply.

> +        for (i = 0; qlit->value.qdict[i].value.type != QTYPE_NONE; i++) {
> +            QLitDictEntry *e = &qlit->value.qdict[i];

I'd prefer the laconic

        QDict *qdict = qdict_new();
        QLitDictEntry *e;

        for (e = qlit->value.qdict; e->key; e++) {
            qdict_put_obj(qdict, e->key, qobject_from_qlit(&e->value));
        }
        return QOBJECT(qdict);

Can slot that in when I apply.

> +
> +            qdict_put_obj(qdict, e->key, qobject_from_qlit(&e->value));
> +        }
> +        return QOBJECT(qdict);
> +    }
> +    case QTYPE_QLIST: {
> +        QList *qlist = qlist_new();
> +
> +        for (i = 0; qlit->value.qlist[i].type != QTYPE_NONE; i++) {
> +            qlist_append_obj(qlist, qobject_from_qlit(&qlit->value.qlist[i]));
> +        }
> +        return QOBJECT(qlist);

Likewise

        QList *qlist = qlist_new();
        QLitObject *e;

        for (e = qlit->value.qlist; e->type != QTYPE_NONE; e++) {
            qlist_append_obj(qlist, qobject_from_qlit(e));
        }
        return QOBJECT(qlist);

> +    }
> +    case QTYPE_QBOOL:
> +        return QOBJECT(qbool_from_bool(qlit->value.qbool));
> +    case QTYPE_NONE:
> +        assert(0);
> +    }
> +
> +    return NULL;
> +}
> diff --git a/tests/check-qlit.c b/tests/check-qlit.c
> index ae74bfcb83..135a399c6d 100644
> --- a/tests/check-qlit.c
> +++ b/tests/check-qlit.c
> @@ -64,11 +64,37 @@ static void qlit_equal_qobject_test(void)
>      qobject_decref(qobj);
>  }
>  
> +static void qobject_from_qlit_test(void)
> +{
> +    QObject *obj, *qobj = qobject_from_qlit(&qlit);
> +    QDict *qdict;
> +    QList *bee;
> +
> +    qdict = qobject_to_qdict(qobj);
> +    g_assert_cmpint(qdict_get_int(qdict, "foo"), ==, 42);
> +    g_assert_cmpstr(qdict_get_str(qdict, "bar"), ==, "hello world");
> +    g_assert(qobject_type(qdict_get(qdict, "baz")) == QTYPE_QNULL);
> +
> +    bee = qdict_get_qlist(qdict, "bee");
> +    obj = qlist_pop(bee);
> +    g_assert_cmpint(qnum_get_int(qobject_to_qnum(obj)), ==, 43);
> +    qobject_decref(obj);
> +    obj = qlist_pop(bee);
> +    g_assert_cmpint(qnum_get_int(qobject_to_qnum(obj)), ==, 44);
> +    qobject_decref(obj);
> +    obj = qlist_pop(bee);
> +    g_assert(qbool_get_bool(qobject_to_qbool(obj)));
> +    qobject_decref(obj);
> +
> +    qobject_decref(qobj);
> +}
> +
>  int main(int argc, char **argv)
>  {
>      g_test_init(&argc, &argv, NULL);
>  
>      g_test_add_func("/qlit/equal_qobject", qlit_equal_qobject_test);
> +    g_test_add_func("/qlit/qobject_from_qlit", qobject_from_qlit_test);
>  
>      return g_test_run();
>  }

With the typo fixed and preferably with the improvements I suggested:
Reviewed-by: Markus Armbruster <armbru@redhat.com>

Re: [Qemu-devel] [PATCH v2 13/14] qlit: add qobject_form_qlit()

[Marc-André Lureau]

Hi

----- Original Message -----
> Eric already pointed out the typo in the title.
> 
> Marc-André Lureau <marcandre.lureau@redhat.com> writes:
> 
> > Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> > ---
> >  include/qapi/qmp/qlit.h |  2 ++
> >  qobject/qlit.c          | 37 +++++++++++++++++++++++++++++++++++++
> >  tests/check-qlit.c      | 26 ++++++++++++++++++++++++++
> >  3 files changed, 65 insertions(+)
> >
> > diff --git a/include/qapi/qmp/qlit.h b/include/qapi/qmp/qlit.h
> > index b18406bce9..56feb25e04 100644
> > --- a/include/qapi/qmp/qlit.h
> > +++ b/include/qapi/qmp/qlit.h
> > @@ -51,4 +51,6 @@ struct QLitDictEntry {
> >  
> >  bool qlit_equal_qobject(const QLitObject *lhs, const QObject *rhs);
> >  
> > +QObject *qobject_from_qlit(const QLitObject *qlit);
> > +
> >  #endif /* QLIT_H */
> > diff --git a/qobject/qlit.c b/qobject/qlit.c
> > index 3c4882c784..e44c09e9eb 100644
> > --- a/qobject/qlit.c
> > +++ b/qobject/qlit.c
> > @@ -82,3 +82,40 @@ bool qlit_equal_qobject(const QLitObject *lhs, const
> > QObject *rhs)
> >  
> >      return false;
> >  }
> > +
> > +QObject *qobject_from_qlit(const QLitObject *qlit)
> > +{
> > +    int i;
> > +
> > +    switch (qlit->type) {
> > +    case QTYPE_QNULL:
> > +        return QOBJECT(qnull());
> > +    case QTYPE_QNUM:
> > +        return QOBJECT(qnum_from_int(qlit->value.qnum));
> 
> This line shows that QLitObject member value.qnum should really be named
> value.qint.  Let's not worry about that now.
> 
> > +    case QTYPE_QSTRING:
> > +        return QOBJECT(qstring_from_str(qlit->value.qstr));
> > +    case QTYPE_QDICT: {
> > +        QDict *qdict = qdict_new();
> 
> Blank line here.  Can touch up when I apply.
> 
> > +        for (i = 0; qlit->value.qdict[i].value.type != QTYPE_NONE; i++) {
> > +            QLitDictEntry *e = &qlit->value.qdict[i];
> 
> I'd prefer the laconic
> 
>         QDict *qdict = qdict_new();
>         QLitDictEntry *e;
> 
>         for (e = qlit->value.qdict; e->key; e++) {
>             qdict_put_obj(qdict, e->key, qobject_from_qlit(&e->value));
>         }
>         return QOBJECT(qdict);
> 
> Can slot that in when I apply.
> 

Indeed!

> > +
> > +            qdict_put_obj(qdict, e->key, qobject_from_qlit(&e->value));
> > +        }
> > +        return QOBJECT(qdict);
> > +    }
> > +    case QTYPE_QLIST: {
> > +        QList *qlist = qlist_new();
> > +
> > +        for (i = 0; qlit->value.qlist[i].type != QTYPE_NONE; i++) {
> > +            qlist_append_obj(qlist,
> > qobject_from_qlit(&qlit->value.qlist[i]));
> > +        }
> > +        return QOBJECT(qlist);
> 
> Likewise
> 
>         QList *qlist = qlist_new();
>         QLitObject *e;
> 
>         for (e = qlit->value.qlist; e->type != QTYPE_NONE; e++) {
>             qlist_append_obj(qlist, qobject_from_qlit(e));
>         }
>         return QOBJECT(qlist);
> 
> > +    }
> > +    case QTYPE_QBOOL:
> > +        return QOBJECT(qbool_from_bool(qlit->value.qbool));
> > +    case QTYPE_NONE:
> > +        assert(0);
> > +    }
> > +
> > +    return NULL;
> > +}
> > diff --git a/tests/check-qlit.c b/tests/check-qlit.c
> > index ae74bfcb83..135a399c6d 100644
> > --- a/tests/check-qlit.c
> > +++ b/tests/check-qlit.c
> > @@ -64,11 +64,37 @@ static void qlit_equal_qobject_test(void)
> >      qobject_decref(qobj);
> >  }
> >  
> > +static void qobject_from_qlit_test(void)
> > +{
> > +    QObject *obj, *qobj = qobject_from_qlit(&qlit);
> > +    QDict *qdict;
> > +    QList *bee;
> > +
> > +    qdict = qobject_to_qdict(qobj);
> > +    g_assert_cmpint(qdict_get_int(qdict, "foo"), ==, 42);
> > +    g_assert_cmpstr(qdict_get_str(qdict, "bar"), ==, "hello world");
> > +    g_assert(qobject_type(qdict_get(qdict, "baz")) == QTYPE_QNULL);
> > +
> > +    bee = qdict_get_qlist(qdict, "bee");
> > +    obj = qlist_pop(bee);
> > +    g_assert_cmpint(qnum_get_int(qobject_to_qnum(obj)), ==, 43);
> > +    qobject_decref(obj);
> > +    obj = qlist_pop(bee);
> > +    g_assert_cmpint(qnum_get_int(qobject_to_qnum(obj)), ==, 44);
> > +    qobject_decref(obj);
> > +    obj = qlist_pop(bee);
> > +    g_assert(qbool_get_bool(qobject_to_qbool(obj)));
> > +    qobject_decref(obj);
> > +
> > +    qobject_decref(qobj);
> > +}
> > +
> >  int main(int argc, char **argv)
> >  {
> >      g_test_init(&argc, &argv, NULL);
> >  
> >      g_test_add_func("/qlit/equal_qobject", qlit_equal_qobject_test);
> > +    g_test_add_func("/qlit/qobject_from_qlit", qobject_from_qlit_test);
> >  
> >      return g_test_run();
> >  }
> 
> With the typo fixed and preferably with the improvements I suggested:
> Reviewed-by: Markus Armbruster <armbru@redhat.com>

Thanks

[Qemu-devel] [PATCH v2 14/14] qapi: generate a literal qobject for introspection

[Marc-André Lureau]

Replace the generated json string with a literal qobject. The later is
easier to deal with, at run time as well as compile time: adding #if
conditionals will be easier than in a json string.

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
---
 scripts/qapi-introspect.py         | 87 ++++++++++++++++++++++----------------
 monitor.c                          |  2 +-
 tests/test-qobject-input-visitor.c | 11 +++--
 docs/devel/qapi-code-gen.txt       | 29 ++++++++-----
 4 files changed, 78 insertions(+), 51 deletions(-)

diff --git a/scripts/qapi-introspect.py b/scripts/qapi-introspect.py
index 032bcea491..a157a5f91c 100644
--- a/scripts/qapi-introspect.py
+++ b/scripts/qapi-introspect.py
@@ -12,31 +12,41 @@
 from qapi import *
 
 
-# Caveman's json.dumps() replacement (we're stuck at Python 2.4)
-# TODO try to use json.dumps() once we get unstuck
-def to_json(obj, level=0):
+def to_qlit(obj, level=0, suppress_first_indent=False):
+
+    def indent(level):
+        return level * 4 * ' '
+
+    ret = ''
+    if not suppress_first_indent:
+        ret += indent(level)
     if obj is None:
-        ret = 'null'
+        ret += 'QLIT_QNULL'
     elif isinstance(obj, str):
-        ret = '"' + obj.replace('"', r'\"') + '"'
+        ret += 'QLIT_QSTR(' + to_c_string(obj) + ')'
     elif isinstance(obj, list):
-        elts = [to_json(elt, level + 1)
+        elts = [to_qlit(elt, level + 1)
                 for elt in obj]
-        ret = '[' + ', '.join(elts) + ']'
+        elts.append(indent(level + 1) + "{}")
+        ret += 'QLIT_QLIST(((QLitObject[]) {\n'
+        ret += ',\n'.join(elts) + '\n'
+        ret += indent(level) + '}))'
     elif isinstance(obj, dict):
-        elts = ['"%s": %s' % (key.replace('"', r'\"'),
-                              to_json(obj[key], level + 1))
-                for key in sorted(obj.keys())]
-        ret = '{' + ', '.join(elts) + '}'
+        elts = []
+        for key, value in sorted(obj.iteritems()):
+            elts.append(indent(level + 1) + '{ %s, %s }' %
+                        (to_c_string(key), to_qlit(value, level + 1, False)))
+        elts.append(indent(level + 1) + '{}')
+        ret += 'QLIT_QDICT(((QLitDictEntry[]) {\n'
+        ret += ',\n'.join(elts) + '\n'
+        ret += indent(level) + '}))'
     else:
         assert False                # not implemented
-    if level == 1:
-        ret = '\n' + ret
     return ret
 
 
-def to_c_string(string):
-    return '"' + string.replace('\\', r'\\').replace('"', r'\"') + '"'
+def to_c_string(s):
+    return '"' + s.replace('\\', r'\\').replace('"', r'\"') + '"'
 
 
 class QAPISchemaGenIntrospectVisitor(QAPISchemaVisitor):
@@ -45,39 +55,37 @@ class QAPISchemaGenIntrospectVisitor(QAPISchemaVisitor):
         self.defn = None
         self.decl = None
         self._schema = None
-        self._jsons = None
+        self._qlits = None
         self._used_types = None
         self._name_map = None
 
     def visit_begin(self, schema):
         self._schema = schema
-        self._jsons = []
+        self._qlits = []
         self._used_types = []
         self._name_map = {}
 
     def visit_end(self):
         # visit the types that are actually used
-        jsons = self._jsons
-        self._jsons = []
+        qlits = self._qlits
+        self._qlits = []
         for typ in self._used_types:
             typ.visit(self)
         # generate C
         # TODO can generate awfully long lines
-        jsons.extend(self._jsons)
-        name = c_name(prefix, protect=False) + 'qmp_schema_json'
+        qlits.extend(self._qlits)
+        name = c_name(prefix, protect=False) + 'qmp_schema_qlit'
         self.decl = mcgen('''
-extern const char %(c_name)s[];
+extern const QLitObject %(c_name)s;
 ''',
                           c_name=c_name(name))
-        lines = to_json(jsons).split('\n')
-        c_string = '\n    '.join([to_c_string(line) for line in lines])
         self.defn = mcgen('''
-const char %(c_name)s[] = %(c_string)s;
+const QLitObject %(c_name)s = %(c_string)s;
 ''',
                           c_name=c_name(name),
-                          c_string=c_string)
+                          c_string=to_qlit(qlits))
         self._schema = None
-        self._jsons = None
+        self._qlits = None
         self._used_types = None
         self._name_map = None
 
@@ -111,12 +119,12 @@ const char %(c_name)s[] = %(c_string)s;
             return '[' + self._use_type(typ.element_type) + ']'
         return self._name(typ.name)
 
-    def _gen_json(self, name, mtype, obj):
+    def _gen_qlit(self, name, mtype, obj):
         if mtype not in ('command', 'event', 'builtin', 'array'):
             name = self._name(name)
         obj['name'] = name
         obj['meta-type'] = mtype
-        self._jsons.append(obj)
+        self._qlits.append(obj)
 
     def _gen_member(self, member):
         ret = {'name': member.name, 'type': self._use_type(member.type)}
@@ -132,24 +140,24 @@ const char %(c_name)s[] = %(c_string)s;
         return {'case': variant.name, 'type': self._use_type(variant.type)}
 
     def visit_builtin_type(self, name, info, json_type):
-        self._gen_json(name, 'builtin', {'json-type': json_type})
+        self._gen_qlit(name, 'builtin', {'json-type': json_type})
 
     def visit_enum_type(self, name, info, values, prefix):
-        self._gen_json(name, 'enum', {'values': values})
+        self._gen_qlit(name, 'enum', {'values': values})
 
     def visit_array_type(self, name, info, element_type):
         element = self._use_type(element_type)
-        self._gen_json('[' + element + ']', 'array', {'element-type': element})
+        self._gen_qlit('[' + element + ']', 'array', {'element-type': element})
 
     def visit_object_type_flat(self, name, info, members, variants):
         obj = {'members': [self._gen_member(m) for m in members]}
         if variants:
             obj.update(self._gen_variants(variants.tag_member.name,
                                           variants.variants))
-        self._gen_json(name, 'object', obj)
+        self._gen_qlit(name, 'object', obj)
 
     def visit_alternate_type(self, name, info, variants):
-        self._gen_json(name, 'alternate',
+        self._gen_qlit(name, 'alternate',
                        {'members': [{'type': self._use_type(m.type)}
                                     for m in variants.variants]})
 
@@ -157,13 +165,13 @@ const char %(c_name)s[] = %(c_string)s;
                       gen, success_response, boxed):
         arg_type = arg_type or self._schema.the_empty_object_type
         ret_type = ret_type or self._schema.the_empty_object_type
-        self._gen_json(name, 'command',
+        self._gen_qlit(name, 'command',
                        {'arg-type': self._use_type(arg_type),
                         'ret-type': self._use_type(ret_type)})
 
     def visit_event(self, name, info, arg_type, boxed):
         arg_type = arg_type or self._schema.the_empty_object_type
-        self._gen_json(name, 'event', {'arg-type': self._use_type(arg_type)})
+        self._gen_qlit(name, 'event', {'arg-type': self._use_type(arg_type)})
 
 # Debugging aid: unmask QAPI schema's type names
 # We normally mask them, because they're not QMP wire ABI
@@ -205,11 +213,18 @@ h_comment = '''
 
 fdef.write(mcgen('''
 #include "qemu/osdep.h"
+#include "qapi/qmp/qlit.h"
 #include "%(prefix)sqmp-introspect.h"
 
 ''',
                  prefix=prefix))
 
+fdecl.write(mcgen('''
+#include "qemu/osdep.h"
+#include "qapi/qmp/qlit.h"
+
+'''))
+
 schema = QAPISchema(input_file)
 gen = QAPISchemaGenIntrospectVisitor(opt_unmask)
 schema.visit(gen)
diff --git a/monitor.c b/monitor.c
index e0f880107f..14c27d4b6f 100644
--- a/monitor.c
+++ b/monitor.c
@@ -953,7 +953,7 @@ EventInfoList *qmp_query_events(Error **errp)
 static void qmp_query_qmp_schema(QDict *qdict, QObject **ret_data,
                                  Error **errp)
 {
-    *ret_data = qobject_from_json(qmp_schema_json, &error_abort);
+    *ret_data = qobject_from_qlit(&qmp_schema_qlit);
 }
 
 /*
diff --git a/tests/test-qobject-input-visitor.c b/tests/test-qobject-input-visitor.c
index bcf02617dc..bdd00f6bd8 100644
--- a/tests/test-qobject-input-visitor.c
+++ b/tests/test-qobject-input-visitor.c
@@ -1247,24 +1247,27 @@ static void test_visitor_in_fail_alternate(TestInputVisitorData *data,
 }
 
 static void do_test_visitor_in_qmp_introspect(TestInputVisitorData *data,
-                                              const char *schema_json)
+                                              const QLitObject *qlit)
 {
     SchemaInfoList *schema = NULL;
+    QObject *obj = qobject_from_qlit(qlit);
     Visitor *v;
 
-    v = visitor_input_test_init_raw(data, schema_json);
+    v = qobject_input_visitor_new(obj);
 
     visit_type_SchemaInfoList(v, NULL, &schema, &error_abort);
     g_assert(schema);
 
     qapi_free_SchemaInfoList(schema);
+    qobject_decref(obj);
+    visit_free(v);
 }
 
 static void test_visitor_in_qmp_introspect(TestInputVisitorData *data,
                                            const void *unused)
 {
-    do_test_visitor_in_qmp_introspect(data, test_qmp_schema_json);
-    do_test_visitor_in_qmp_introspect(data, qmp_schema_json);
+    do_test_visitor_in_qmp_introspect(data, &test_qmp_schema_qlit);
+    do_test_visitor_in_qmp_introspect(data, &qmp_schema_qlit);
 }
 
 int main(int argc, char **argv)
diff --git a/docs/devel/qapi-code-gen.txt b/docs/devel/qapi-code-gen.txt
index 9903ac4c19..b653e86bff 100644
--- a/docs/devel/qapi-code-gen.txt
+++ b/docs/devel/qapi-code-gen.txt
@@ -1295,18 +1295,27 @@ Example:
     #ifndef EXAMPLE_QMP_INTROSPECT_H
     #define EXAMPLE_QMP_INTROSPECT_H
 
-    extern const char example_qmp_schema_json[];
+    extern const QLitObject qmp_schema_qlit;
 
     #endif
     $ cat qapi-generated/example-qmp-introspect.c
 [Uninteresting stuff omitted...]
 
-    const char example_qmp_schema_json[] = "["
-        "{\"arg-type\": \"0\", \"meta-type\": \"event\", \"name\": \"MY_EVENT\"}, "
-        "{\"arg-type\": \"1\", \"meta-type\": \"command\", \"name\": \"my-command\", \"ret-type\": \"2\"}, "
-        "{\"members\": [], \"meta-type\": \"object\", \"name\": \"0\"}, "
-        "{\"members\": [{\"name\": \"arg1\", \"type\": \"[2]\"}], \"meta-type\": \"object\", \"name\": \"1\"}, "
-        "{\"members\": [{\"name\": \"integer\", \"type\": \"int\"}, {\"default\": null, \"name\": \"string\", \"type\": \"str\"}], \"meta-type\": \"object\", \"name\": \"2\"}, "
-        "{\"element-type\": \"2\", \"meta-type\": \"array\", \"name\": \"[2]\"}, "
-        "{\"json-type\": \"int\", \"meta-type\": \"builtin\", \"name\": \"int\"}, "
-        "{\"json-type\": \"string\", \"meta-type\": \"builtin\", \"name\": \"str\"}]";
+    const QLitObject example_qmp_schema_qlit = QLIT_QLIST(((QLitObject[]) {
+        QLIT_QDICT(((QLitDictEntry[]) {
+            { "arg-type", QLIT_QSTR("0") },
+            { "meta-type", QLIT_QSTR("event") },
+            { "name", QLIT_QSTR("Event") },
+            { }
+        })),
+        QLIT_QDICT(((QLitDictEntry[]) {
+            { "members", QLIT_QLIST(((QLitObject[]) {
+                { }
+            })) },
+            { "meta-type", QLIT_QSTR("object") },
+            { "name", QLIT_QSTR("0") },
+            { }
+        })),
+        ...
+        { }
+    }));
-- 
2.14.1.146.gd35faa819

Re: [Qemu-devel] [PATCH v2 00/14] Generate a literal qobject for introspection

[Markus Armbruster]

Marc-André Lureau <marcandre.lureau@redhat.com> writes:

> Hi,
>
> This series is based on patches 2-5 from "[PATCH v2 00/54] qapi: add #if
> pre-processor conditions to generated code". Generating a literal qobject
> will allow to easily introduce #if conditionals from the original series.
>
> v2:
> - use QLIST_FOREACH_ENTRY for list comparison
> - update qdict comparison to check for the size instead of doing a
>   qdict copy
> - commit message update
> - add r-b tags
>
> Marc-André Lureau (14):
>   qdict: add qdict_put_null() helper
>   qlit: move qlit from check-qjson to qobject/
>   qlit: use QLit prefix consistently
>   qlit: remove compound literals
>   qlit: rename compare_litqobj_to_qobj() to qlit_equal_qobject()
>   qlit: make qlit_equal_qobject return a bool
>   qlit: make qlit_equal_qobject() take const arguments
>   qlit: add QLIT_QNULL and QLIT_BOOL
>   qlit: Replace open-coded qnum_get_int() by call
>   tests: add qlit tests
>   qlit: improve QLit dict vs qdict comparison
>   qlit: improve QLit list vs qlist comparison
>   qlit: add qobject_form_qlit()
>   qapi: generate a literal qobject for introspection

PATCH 14 makes sense only together with work mentioned in its commit
message.  Without PATCH 14, PATCH 13 is left without a user.
Fortunately, PATCH 01-12 make sense on their own: applied to qapi-next.
PATCH 13-14 applied to qapi-not-next, to give you a suitable base for
respins of the remainder of "qapi: add #if pre-processor conditions to
generated code".