[PATCH v3 00/19] qom: Use qlit to represent property defaults

[PATCH v3 00/19] qom: Use qlit to represent property defaults

[Eduardo Habkost]

Based-on: 20201104160021.2342108-1-ehabkost@redhat.com
Git branch: https://gitlab.com/ehabkost/qemu/-/commits/work/qdev-qlit-defaults

This extend qlit.h to support all QNum types (signed int,
unsigned int, and double), and use QLitObject to represent field
property defaults.

It allows us to get rid of most type-specific .set_default_value
functions for QOM property types.

The only remaining .set_default_value function in the code is
field_prop_set_default_value_enum().  This will take a bit more
work because the default is currently stored as int, but parsed
as string, so it will be addressed in a separate series.

Changes v2 -> v3:

* qnum/qlit patches:
  * Variable naming in qnum code
  * Coding style changes in qnum code
  * Split "qlit: Support all types of QNums"
    in smaller patches
  * Replace QLIT_QNUM with QLIT_QNUM_INT
  * Extend test cases, move most of the new test case code to
    check-qlit.c
  * Remove qnum_get_value() and qlit_get_type() function
  * Removed kernel-doc conversion patch, to reduce noise in series review
* qom patches:
  * Split "qom: Use qlit to represent property defaults" in
    smaller patches, hopefully making review easier
  * Small changes in UUID property code (See
    "qom: Don't ignore defval on UUID property" and
    "qom: Fix documentation of UUID property type")

Changes v1 -> v2:
* Rebase to latest version of field properties series
* Fix unit test failure
* Coding style changes

Eduardo Habkost (19):
  qnum: Make qnum_get_double() get const pointer
  qnum: Make num_x/num_y variables at qnum_is_equal() const
  qnum: QNumValue type for QNum value literals
  qnum: qnum_value_is_equal() function
  qlit: Use qnum_value_is_equal() when comparing QNums
  qlit: Rename QLIT_QNUM to QLIT_QNUM_INT
  qlit: Use QNumValue to represent QNums
  qlit: Move qlit_equal_qobject() reference values to array
  qlit: Add more test literals to qlit_equal_qobject() test case
  qlit: Support all types of QNums
  qom: field_prop_set_default_value() helper
  qom: Replace defval value in Property with QLitObject
  qom: Fix documentation of UUID property type
  qom: Don't ignore defval on UUID property
  qom: Make object_property_set_default() public
  qom: Make PropertyInfo.set_default_value optional
  qom: Delete field_prop_set_default_value_*int()
  qom: Delete set_default_uuid()
  qom: Delete set_default_value_bool()

 include/hw/qdev-properties-system.h   |   4 +-
 include/qapi/qmp/qlit.h               |  11 ++-
 include/qapi/qmp/qnum.h               |  26 +++++-
 include/qom/field-property-internal.h |   7 +-
 include/qom/field-property.h          |  30 +++----
 include/qom/object.h                  |  11 +++
 include/qom/property-types.h          |  18 ++--
 hw/core/qdev-properties-system.c      |  13 +--
 qobject/qlit.c                        |   5 +-
 qobject/qnum.c                        | 113 ++++++++++++++------------
 qom/field-property.c                  |  36 ++++++--
 qom/object.c                          |   2 +-
 qom/property-types.c                  |  37 ++-------
 tests/check-qjson.c                   |  30 +++----
 tests/check-qlit.c                    |  72 +++++++++++++---
 tests/check-qnum.c                    |  14 ++--
 16 files changed, 253 insertions(+), 176 deletions(-)

-- 
2.28.0

[PATCH v3 01/19] qnum: Make qnum_get_double() get const pointer

[Eduardo Habkost]

qnum_get_double() won't change the object, the argument can be
const.

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

diff --git a/include/qapi/qmp/qnum.h b/include/qapi/qmp/qnum.h
index bbae0a5ec8..3e9ecd324e 100644
--- a/include/qapi/qmp/qnum.h
+++ b/include/qapi/qmp/qnum.h
@@ -64,7 +64,7 @@ int64_t qnum_get_int(const QNum *qn);
 bool qnum_get_try_uint(const QNum *qn, uint64_t *val);
 uint64_t qnum_get_uint(const QNum *qn);
 
-double qnum_get_double(QNum *qn);
+double qnum_get_double(const QNum *qn);
 
 char *qnum_to_string(QNum *qn);
 
diff --git a/qobject/qnum.c b/qobject/qnum.c
index 7012fc57f2..d328d91fcb 100644
--- a/qobject/qnum.c
+++ b/qobject/qnum.c
@@ -144,7 +144,7 @@ uint64_t qnum_get_uint(const QNum *qn)
  *
  * qnum_get_double() loses precision for integers beyond 53 bits.
  */
-double qnum_get_double(QNum *qn)
+double qnum_get_double(const QNum *qn)
 {
     switch (qn->kind) {
     case QNUM_I64:
-- 
2.28.0

[PATCH v3 02/19] qnum: Make num_x/num_y variables at qnum_is_equal() const

[Eduardo Habkost]

qobject_to() drops const-ness by accident, but our function
arguments (x, y) are const.  Make num_x/num_y const too.

Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
This is a new patch added in v3 of the series.
---
 qobject/qnum.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/qobject/qnum.c b/qobject/qnum.c
index d328d91fcb..e5ea728638 100644
--- a/qobject/qnum.c
+++ b/qobject/qnum.c
@@ -209,8 +209,8 @@ char *qnum_to_string(QNum *qn)
  */
 bool qnum_is_equal(const QObject *x, const QObject *y)
 {
-    QNum *num_x = qobject_to(QNum, x);
-    QNum *num_y = qobject_to(QNum, y);
+    const QNum *num_x = qobject_to(QNum, x);
+    const QNum *num_y = qobject_to(QNum, y);
 
     switch (num_x->kind) {
     case QNUM_I64:
-- 
2.28.0

[PATCH v3 03/19] qnum: QNumValue type for QNum value literals

[Eduardo Habkost]

Provide a separate QNumValue type that can be used for QNum value
literals without the referencing counting and memory allocation
features provided by QObject.

Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
Changes v2 -> v3:
* Fixed copy-pasta at qnum_from_value() documentation
* Removed qnum_get_value() function
* Moved doc comment of qnum_from_value() to .c file, for
  consistency with other functions.
* Removed "private:" doc comment at QNumValue.
* Removed unnecessary kernel-doc noise (obvious
  parameter descriptions).
* Removed space after type cast in qnum_from_*().
* qnum_is_equal() variable const-ness & renames:
  * Renamed new QNumValue variables to val_x/val_y.
  * Keep existing QNum num_x/num_y variable names.
  * const-ness change of num_x/num_y was moved to a separate
    patch.

Changes v1 -> v2:
* Fix "make check" failure, by updating check-qnum unit test to
  use the new struct fields
---
 include/qapi/qmp/qnum.h | 23 ++++++++++-
 qobject/qnum.c          | 91 ++++++++++++++++++++++-------------------
 tests/check-qnum.c      | 14 +++----
 3 files changed, 76 insertions(+), 52 deletions(-)

diff --git a/include/qapi/qmp/qnum.h b/include/qapi/qmp/qnum.h
index 3e9ecd324e..03193dca20 100644
--- a/include/qapi/qmp/qnum.h
+++ b/include/qapi/qmp/qnum.h
@@ -44,16 +44,35 @@ typedef enum {
  * in range: qnum_get_try_int() / qnum_get_try_uint() check range and
  * convert under the hood.
  */
-struct QNum {
-    struct QObjectBase_ base;
+
+/**
+ * struct QNumValue: the value of a QNum
+ *
+ * QNumValue literals can be constructed using the `QNUM_VAL_INT`,
+ * `QNUM_VAL_UINT`, and `QNUM_VAL_DOUBLE` macros.
+ */
+typedef struct QNumValue {
     QNumKind kind;
     union {
         int64_t i64;
         uint64_t u64;
         double dbl;
     } u;
+} QNumValue;
+
+#define QNUM_VAL_INT(value) \
+    { .kind = QNUM_I64, .u.i64 = value }
+#define QNUM_VAL_UINT(value) \
+    { .kind = QNUM_U64, .u.u64 = value }
+#define QNUM_VAL_DOUBLE(value) \
+    { .kind = QNUM_DOUBLE, .u.dbl = value }
+
+struct QNum {
+    struct QObjectBase_ base;
+    QNumValue value;
 };
 
+QNum *qnum_from_value(QNumValue value);
 QNum *qnum_from_int(int64_t value);
 QNum *qnum_from_uint(uint64_t value);
 QNum *qnum_from_double(double value);
diff --git a/qobject/qnum.c b/qobject/qnum.c
index e5ea728638..94e668db60 100644
--- a/qobject/qnum.c
+++ b/qobject/qnum.c
@@ -16,21 +16,29 @@
 #include "qapi/qmp/qnum.h"
 
 /**
- * qnum_from_int(): Create a new QNum from an int64_t
+ * qnum_from_value(): Create a new QNum from a QNumValue
  *
  * Return strong reference.
  */
-QNum *qnum_from_int(int64_t value)
+QNum *qnum_from_value(QNumValue value)
 {
     QNum *qn = g_new(QNum, 1);
 
     qobject_init(QOBJECT(qn), QTYPE_QNUM);
-    qn->kind = QNUM_I64;
-    qn->u.i64 = value;
-
+    qn->value = value;
     return qn;
 }
 
+/**
+ * qnum_from_int(): Create a new QNum from an int64_t
+ *
+ * Return strong reference.
+ */
+QNum *qnum_from_int(int64_t value)
+{
+    return qnum_from_value((QNumValue)QNUM_VAL_INT(value));
+}
+
 /**
  * qnum_from_uint(): Create a new QNum from an uint64_t
  *
@@ -38,13 +46,7 @@ QNum *qnum_from_int(int64_t value)
  */
 QNum *qnum_from_uint(uint64_t value)
 {
-    QNum *qn = g_new(QNum, 1);
-
-    qobject_init(QOBJECT(qn), QTYPE_QNUM);
-    qn->kind = QNUM_U64;
-    qn->u.u64 = value;
-
-    return qn;
+    return qnum_from_value((QNumValue)QNUM_VAL_UINT(value));
 }
 
 /**
@@ -54,13 +56,7 @@ QNum *qnum_from_uint(uint64_t value)
  */
 QNum *qnum_from_double(double value)
 {
-    QNum *qn = g_new(QNum, 1);
-
-    qobject_init(QOBJECT(qn), QTYPE_QNUM);
-    qn->kind = QNUM_DOUBLE;
-    qn->u.dbl = value;
-
-    return qn;
+    return qnum_from_value((QNumValue)QNUM_VAL_DOUBLE(value));
 }
 
 /**
@@ -70,15 +66,17 @@ QNum *qnum_from_double(double value)
  */
 bool qnum_get_try_int(const QNum *qn, int64_t *val)
 {
-    switch (qn->kind) {
+    const QNumValue *qv = &qn->value;
+
+    switch (qv->kind) {
     case QNUM_I64:
-        *val = qn->u.i64;
+        *val = qv->u.i64;
         return true;
     case QNUM_U64:
-        if (qn->u.u64 > INT64_MAX) {
+        if (qv->u.u64 > INT64_MAX) {
             return false;
         }
-        *val = qn->u.u64;
+        *val = qv->u.u64;
         return true;
     case QNUM_DOUBLE:
         return false;
@@ -108,15 +106,17 @@ int64_t qnum_get_int(const QNum *qn)
  */
 bool qnum_get_try_uint(const QNum *qn, uint64_t *val)
 {
-    switch (qn->kind) {
+    const QNumValue *qv = &qn->value;
+
+    switch (qv->kind) {
     case QNUM_I64:
-        if (qn->u.i64 < 0) {
+        if (qv->u.i64 < 0) {
             return false;
         }
-        *val = qn->u.i64;
+        *val = qv->u.i64;
         return true;
     case QNUM_U64:
-        *val = qn->u.u64;
+        *val = qv->u.u64;
         return true;
     case QNUM_DOUBLE:
         return false;
@@ -146,13 +146,15 @@ uint64_t qnum_get_uint(const QNum *qn)
  */
 double qnum_get_double(const QNum *qn)
 {
-    switch (qn->kind) {
+    const QNumValue *qv = &qn->value;
+
+    switch (qv->kind) {
     case QNUM_I64:
-        return qn->u.i64;
+        return qv->u.i64;
     case QNUM_U64:
-        return qn->u.u64;
+        return qv->u.u64;
     case QNUM_DOUBLE:
-        return qn->u.dbl;
+        return qv->u.dbl;
     }
 
     assert(0);
@@ -161,14 +163,15 @@ double qnum_get_double(const QNum *qn)
 
 char *qnum_to_string(QNum *qn)
 {
+    const QNumValue *qv = &qn->value;
     char *buffer;
     int len;
 
-    switch (qn->kind) {
+    switch (qv->kind) {
     case QNUM_I64:
-        return g_strdup_printf("%" PRId64, qn->u.i64);
+        return g_strdup_printf("%" PRId64, qv->u.i64);
     case QNUM_U64:
-        return g_strdup_printf("%" PRIu64, qn->u.u64);
+        return g_strdup_printf("%" PRIu64, qv->u.u64);
     case QNUM_DOUBLE:
         /* FIXME: snprintf() is locale dependent; but JSON requires
          * numbers to be formatted as if in the C locale. Dependence
@@ -179,7 +182,7 @@ char *qnum_to_string(QNum *qn)
          * rounding errors; we should be using DBL_DECIMAL_DIG (17),
          * and only rounding to a shorter number if the result would
          * still produce the same floating point value.  */
-        buffer = g_strdup_printf("%f" , qn->u.dbl);
+        buffer = g_strdup_printf("%f" , qv->u.dbl);
         len = strlen(buffer);
         while (len > 0 && buffer[len - 1] == '0') {
             len--;
@@ -211,40 +214,42 @@ bool qnum_is_equal(const QObject *x, const QObject *y)
 {
     const QNum *num_x = qobject_to(QNum, x);
     const QNum *num_y = qobject_to(QNum, y);
+    const QNumValue *val_x = &num_x->value;
+    const QNumValue *val_y = &num_y->value;
 
-    switch (num_x->kind) {
+    switch (val_x->kind) {
     case QNUM_I64:
-        switch (num_y->kind) {
+        switch (val_y->kind) {
         case QNUM_I64:
             /* Comparison in native int64_t type */
-            return num_x->u.i64 == num_y->u.i64;
+            return val_x->u.i64 == val_y->u.i64;
         case QNUM_U64:
             /* Implicit conversion of x to uin64_t, so we have to
              * check its sign before */
-            return num_x->u.i64 >= 0 && num_x->u.i64 == num_y->u.u64;
+            return val_x->u.i64 >= 0 && val_x->u.i64 == val_y->u.u64;
         case QNUM_DOUBLE:
             return false;
         }
         abort();
     case QNUM_U64:
-        switch (num_y->kind) {
+        switch (val_y->kind) {
         case QNUM_I64:
             return qnum_is_equal(y, x);
         case QNUM_U64:
             /* Comparison in native uint64_t type */
-            return num_x->u.u64 == num_y->u.u64;
+            return val_x->u.u64 == val_y->u.u64;
         case QNUM_DOUBLE:
             return false;
         }
         abort();
     case QNUM_DOUBLE:
-        switch (num_y->kind) {
+        switch (val_y->kind) {
         case QNUM_I64:
         case QNUM_U64:
             return false;
         case QNUM_DOUBLE:
             /* Comparison in native double type */
-            return num_x->u.dbl == num_y->u.dbl;
+            return val_x->u.dbl == val_y->u.dbl;
         }
         abort();
     }
diff --git a/tests/check-qnum.c b/tests/check-qnum.c
index 4105015872..9499b0d845 100644
--- a/tests/check-qnum.c
+++ b/tests/check-qnum.c
@@ -30,8 +30,8 @@ static void qnum_from_int_test(void)
 
     qn = qnum_from_int(value);
     g_assert(qn != NULL);
-    g_assert_cmpint(qn->kind, ==, QNUM_I64);
-    g_assert_cmpint(qn->u.i64, ==, value);
+    g_assert_cmpint(qn->value.kind, ==, QNUM_I64);
+    g_assert_cmpint(qn->value.u.i64, ==, value);
     g_assert_cmpint(qn->base.refcnt, ==, 1);
     g_assert_cmpint(qobject_type(QOBJECT(qn)), ==, QTYPE_QNUM);
 
@@ -45,8 +45,8 @@ static void qnum_from_uint_test(void)
 
     qn = qnum_from_uint(value);
     g_assert(qn != NULL);
-    g_assert_cmpint(qn->kind, ==, QNUM_U64);
-    g_assert(qn->u.u64 == value);
+    g_assert_cmpint(qn->value.kind, ==, QNUM_U64);
+    g_assert(qn->value.u.u64 == value);
     g_assert(qn->base.refcnt == 1);
     g_assert(qobject_type(QOBJECT(qn)) == QTYPE_QNUM);
 
@@ -60,8 +60,8 @@ static void qnum_from_double_test(void)
 
     qn = qnum_from_double(value);
     g_assert(qn != NULL);
-    g_assert_cmpint(qn->kind, ==, QNUM_DOUBLE);
-    g_assert_cmpfloat(qn->u.dbl, ==, value);
+    g_assert_cmpint(qn->value.kind, ==, QNUM_DOUBLE);
+    g_assert_cmpfloat(qn->value.u.dbl, ==, value);
     g_assert_cmpint(qn->base.refcnt, ==, 1);
     g_assert_cmpint(qobject_type(QOBJECT(qn)), ==, QTYPE_QNUM);
 
@@ -74,7 +74,7 @@ static void qnum_from_int64_test(void)
     const int64_t value = 0x1234567890abcdefLL;
 
     qn = qnum_from_int(value);
-    g_assert_cmpint((int64_t) qn->u.i64, ==, value);
+    g_assert_cmpint((int64_t) qn->value.u.i64, ==, value);
 
     qobject_unref(qn);
 }
-- 
2.28.0

[PATCH v3 04/19] qnum: qnum_value_is_equal() function

[Eduardo Habkost]

Extract the QNum value comparison logic to a function that takes
QNumValue* as argument.

Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
Changes v2 -> v3:
* Rename function parameters to val_x/val_y
* Insert blank line after variable declarations at
  qnum_is_equal()
---
 include/qapi/qmp/qnum.h |  1 +
 qobject/qnum.c          | 24 ++++++++++++++++--------
 2 files changed, 17 insertions(+), 8 deletions(-)

diff --git a/include/qapi/qmp/qnum.h b/include/qapi/qmp/qnum.h
index 03193dca20..3dcb020689 100644
--- a/include/qapi/qmp/qnum.h
+++ b/include/qapi/qmp/qnum.h
@@ -87,6 +87,7 @@ double qnum_get_double(const QNum *qn);
 
 char *qnum_to_string(QNum *qn);
 
+bool qnum_value_is_equal(const QNumValue *val_x, const QNumValue *val_y);
 bool qnum_is_equal(const QObject *x, const QObject *y);
 void qnum_destroy_obj(QObject *obj);
 
diff --git a/qobject/qnum.c b/qobject/qnum.c
index 94e668db60..53c637f46d 100644
--- a/qobject/qnum.c
+++ b/qobject/qnum.c
@@ -202,7 +202,7 @@ char *qnum_to_string(QNum *qn)
 }
 
 /**
- * qnum_is_equal(): Test whether the two QNums are equal
+ * qnum_value_is_equal(): Test whether two QNumValues are equal
  *
  * Negative integers are never considered equal to unsigned integers,
  * but positive integers in the range [0, INT64_MAX] are considered
@@ -210,13 +210,8 @@ char *qnum_to_string(QNum *qn)
  *
  * Doubles are never considered equal to integers.
  */
-bool qnum_is_equal(const QObject *x, const QObject *y)
+bool qnum_value_is_equal(const QNumValue *val_x, const QNumValue *val_y)
 {
-    const QNum *num_x = qobject_to(QNum, x);
-    const QNum *num_y = qobject_to(QNum, y);
-    const QNumValue *val_x = &num_x->value;
-    const QNumValue *val_y = &num_y->value;
-
     switch (val_x->kind) {
     case QNUM_I64:
         switch (val_y->kind) {
@@ -234,7 +229,7 @@ bool qnum_is_equal(const QObject *x, const QObject *y)
     case QNUM_U64:
         switch (val_y->kind) {
         case QNUM_I64:
-            return qnum_is_equal(y, x);
+            return qnum_value_is_equal(val_y, val_x);
         case QNUM_U64:
             /* Comparison in native uint64_t type */
             return val_x->u.u64 == val_y->u.u64;
@@ -257,6 +252,19 @@ bool qnum_is_equal(const QObject *x, const QObject *y)
     abort();
 }
 
+/**
+ * qnum_is_equal(): Test whether the two QNums are equal
+ *
+ * See qnum_value_is_equal() for details on the comparison rules.
+ */
+bool qnum_is_equal(const QObject *x, const QObject *y)
+{
+    const QNum *qnum_x = qobject_to(QNum, x);
+    const QNum *qnum_y = qobject_to(QNum, y);
+
+    return qnum_value_is_equal(&qnum_x->value, &qnum_y->value);
+}
+
 /**
  * qnum_destroy_obj(): Free all memory allocated by a
  * QNum object
-- 
2.28.0

[PATCH v3 05/19] qlit: Use qnum_value_is_equal() when comparing QNums

[Eduardo Habkost]

Currently, qlit_equal_qobject() crashes if getting a QNum that
can't be represented as int64.  Fix this by using
qnum_value_is_equal().

Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
This is a new patch added in v3 of the series.
---
 qobject/qlit.c     |  3 ++-
 tests/check-qlit.c | 19 +++++++++++++++++++
 2 files changed, 21 insertions(+), 1 deletion(-)

diff --git a/qobject/qlit.c b/qobject/qlit.c
index be8332136c..67126b25d5 100644
--- a/qobject/qlit.c
+++ b/qobject/qlit.c
@@ -71,7 +71,8 @@ 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:
-        return lhs->value.qnum ==  qnum_get_int(qobject_to(QNum, rhs));
+        return qnum_value_is_equal(&(QNumValue)QNUM_VAL_INT(lhs->value.qnum),
+                                   &qobject_to(QNum, rhs)->value);
     case QTYPE_QSTRING:
         return (strcmp(lhs->value.qstr,
                        qstring_get_str(qobject_to(QString, rhs))) == 0);
diff --git a/tests/check-qlit.c b/tests/check-qlit.c
index bd6798d912..58ceaae5a3 100644
--- a/tests/check-qlit.c
+++ b/tests/check-qlit.c
@@ -65,6 +65,24 @@ static void qlit_equal_qobject_test(void)
     qobject_unref(qobj);
 }
 
+static void qlit_equal_large_qnum_test(void)
+{
+    /* 2^32-1 */
+    QNum *large = qnum_from_uint(9223372036854775807LL);
+    /* 2^32 */
+    QNum *too_large = qnum_from_uint(9223372036854775808ULL);
+    QNum *dbl = qnum_from_double(9223372036854775808.0);
+    QLitObject qlit_large = QLIT_QNUM(9223372036854775807LL);
+
+    g_assert(qlit_equal_qobject(&qlit_large, QOBJECT(large)));
+    g_assert(!qlit_equal_qobject(&qlit_large, QOBJECT(too_large)));
+    g_assert(!qlit_equal_qobject(&qlit_large, QOBJECT(dbl)));
+
+    qobject_unref(dbl);
+    qobject_unref(large);
+    qobject_unref(too_large);
+}
+
 static void qobject_from_qlit_test(void)
 {
     QObject *obj, *qobj = qobject_from_qlit(&qlit);
@@ -95,6 +113,7 @@ 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/equal_large_qnum", qlit_equal_large_qnum_test);
     g_test_add_func("/qlit/qobject_from_qlit", qobject_from_qlit_test);
 
     return g_test_run();
-- 
2.28.0

[PATCH v3 06/19] qlit: Rename QLIT_QNUM to QLIT_QNUM_INT

[Eduardo Habkost]

Rename the existing QLIT_QNUM macro to indicate it only supports
signed int values.  We're going to add support to other types of
QNums later.

Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
This is a new patch added in v3 of this series.

In v2, the existing QLIT_QNUM() macro was being kept, now it is
replaced by QLIT_QNUM_INT().
---
 include/qapi/qmp/qlit.h |  2 +-
 tests/check-qjson.c     | 30 +++++++++++++++---------------
 tests/check-qlit.c      | 10 +++++-----
 3 files changed, 21 insertions(+), 21 deletions(-)

diff --git a/include/qapi/qmp/qlit.h b/include/qapi/qmp/qlit.h
index c0676d5daf..2fc2db282e 100644
--- a/include/qapi/qmp/qlit.h
+++ b/include/qapi/qmp/qlit.h
@@ -39,7 +39,7 @@ struct QLitDictEntry {
     { .type = QTYPE_QNULL }
 #define QLIT_QBOOL(val) \
     { .type = QTYPE_QBOOL, .value.qbool = (val) }
-#define QLIT_QNUM(val) \
+#define QLIT_QNUM_INT(val) \
     { .type = QTYPE_QNUM, .value.qnum = (val) }
 #define QLIT_QSTR(val) \
     { .type = QTYPE_QSTRING, .value.qstr = (val) }
diff --git a/tests/check-qjson.c b/tests/check-qjson.c
index 07a773e653..bc5b7ebdf3 100644
--- a/tests/check-qjson.c
+++ b/tests/check-qjson.c
@@ -1062,7 +1062,7 @@ static void simple_dict(void)
         {
             .encoded = "{\"foo\": 42, \"bar\": \"hello world\"}",
             .decoded = QLIT_QDICT(((QLitDictEntry[]){
-                        { "foo", QLIT_QNUM(42) },
+                        { "foo", QLIT_QNUM_INT(42) },
                         { "bar", QLIT_QSTR("hello world") },
                         { }
                     })),
@@ -1074,7 +1074,7 @@ static void simple_dict(void)
         }, {
             .encoded = "{\"foo\": 43}",
             .decoded = QLIT_QDICT(((QLitDictEntry[]){
-                        { "foo", QLIT_QNUM(43) },
+                        { "foo", QLIT_QNUM_INT(43) },
                         { }
                     })),
         },
@@ -1160,15 +1160,15 @@ static void simple_list(void)
         {
             .encoded = "[43,42]",
             .decoded = QLIT_QLIST(((QLitObject[]){
-                        QLIT_QNUM(43),
-                        QLIT_QNUM(42),
+                        QLIT_QNUM_INT(43),
+                        QLIT_QNUM_INT(42),
                         { }
                     })),
         },
         {
             .encoded = "[43]",
             .decoded = QLIT_QLIST(((QLitObject[]){
-                        QLIT_QNUM(43),
+                        QLIT_QNUM_INT(43),
                         { }
                     })),
         },
@@ -1217,35 +1217,35 @@ static void simple_whitespace(void)
         {
             .encoded = " [ 43 , 42 ]",
             .decoded = QLIT_QLIST(((QLitObject[]){
-                        QLIT_QNUM(43),
-                        QLIT_QNUM(42),
+                        QLIT_QNUM_INT(43),
+                        QLIT_QNUM_INT(42),
                         { }
                     })),
         },
         {
             .encoded = "\t[ 43 , { 'h' : 'b' },\r\n\t[ ], 42 ]\n",
             .decoded = QLIT_QLIST(((QLitObject[]){
-                        QLIT_QNUM(43),
+                        QLIT_QNUM_INT(43),
                         QLIT_QDICT(((QLitDictEntry[]){
                                     { "h", QLIT_QSTR("b") },
                                     { }})),
                         QLIT_QLIST(((QLitObject[]){
                                     { }})),
-                        QLIT_QNUM(42),
+                        QLIT_QNUM_INT(42),
                         { }
                     })),
         },
         {
             .encoded = " [ 43 , { 'h' : 'b' , 'a' : 32 }, [ ], 42 ]",
             .decoded = QLIT_QLIST(((QLitObject[]){
-                        QLIT_QNUM(43),
+                        QLIT_QNUM_INT(43),
                         QLIT_QDICT(((QLitDictEntry[]){
                                     { "h", QLIT_QSTR("b") },
-                                    { "a", QLIT_QNUM(32) },
+                                    { "a", QLIT_QNUM_INT(32) },
                                     { }})),
                         QLIT_QLIST(((QLitObject[]){
                                     { }})),
-                        QLIT_QNUM(42),
+                        QLIT_QNUM_INT(42),
                         { }
                     })),
         },
@@ -1275,11 +1275,11 @@ static void simple_interpolation(void)
     QObject *embedded_obj;
     QObject *obj;
     QLitObject decoded = QLIT_QLIST(((QLitObject[]){
-            QLIT_QNUM(1),
+            QLIT_QNUM_INT(1),
             QLIT_QSTR("100%"),
             QLIT_QLIST(((QLitObject[]){
-                        QLIT_QNUM(32),
-                        QLIT_QNUM(42),
+                        QLIT_QNUM_INT(32),
+                        QLIT_QNUM_INT(42),
                         {}})),
             {}}));
 
diff --git a/tests/check-qlit.c b/tests/check-qlit.c
index 58ceaae5a3..24ac21395c 100644
--- a/tests/check-qlit.c
+++ b/tests/check-qlit.c
@@ -17,12 +17,12 @@
 #include "qapi/qmp/qstring.h"
 
 static QLitObject qlit = QLIT_QDICT(((QLitDictEntry[]) {
-    { "foo", QLIT_QNUM(42) },
+    { "foo", QLIT_QNUM_INT(42) },
     { "bar", QLIT_QSTR("hello world") },
     { "baz", QLIT_QNULL },
     { "bee", QLIT_QLIST(((QLitObject[]) {
-        QLIT_QNUM(43),
-        QLIT_QNUM(44),
+        QLIT_QNUM_INT(43),
+        QLIT_QNUM_INT(44),
         QLIT_QBOOL(true),
         { },
     }))},
@@ -30,7 +30,7 @@ static QLitObject qlit = QLIT_QDICT(((QLitDictEntry[]) {
 }));
 
 static QLitObject qlit_foo = QLIT_QDICT(((QLitDictEntry[]) {
-    { "foo", QLIT_QNUM(42) },
+    { "foo", QLIT_QNUM_INT(42) },
     { },
 }));
 
@@ -72,7 +72,7 @@ static void qlit_equal_large_qnum_test(void)
     /* 2^32 */
     QNum *too_large = qnum_from_uint(9223372036854775808ULL);
     QNum *dbl = qnum_from_double(9223372036854775808.0);
-    QLitObject qlit_large = QLIT_QNUM(9223372036854775807LL);
+    QLitObject qlit_large = QLIT_QNUM_INT(9223372036854775807LL);
 
     g_assert(qlit_equal_qobject(&qlit_large, QOBJECT(large)));
     g_assert(!qlit_equal_qobject(&qlit_large, QOBJECT(too_large)));
-- 
2.28.0

[PATCH v3 07/19] qlit: Use QNumValue to represent QNums

[Eduardo Habkost]

Replace the existing int64_t field in QLitObject with QNumValue,
so we can get support for other QNum types for free.

Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
This is a new patch added in v3 of the series.  It includes
portions of a previous patch from v2:
"qlit: Support all types of QNums".
---
 include/qapi/qmp/qlit.h | 5 +++--
 qobject/qlit.c          | 4 ++--
 2 files changed, 5 insertions(+), 4 deletions(-)

diff --git a/include/qapi/qmp/qlit.h b/include/qapi/qmp/qlit.h
index 2fc2db282e..a240cdd299 100644
--- a/include/qapi/qmp/qlit.h
+++ b/include/qapi/qmp/qlit.h
@@ -15,6 +15,7 @@
 #define QLIT_H
 
 #include "qobject.h"
+#include "qnum.h"
 
 typedef struct QLitDictEntry QLitDictEntry;
 typedef struct QLitObject QLitObject;
@@ -23,7 +24,7 @@ struct QLitObject {
     QType type;
     union {
         bool qbool;
-        int64_t qnum;
+        QNumValue qnum;
         const char *qstr;
         QLitDictEntry *qdict;
         QLitObject *qlist;
@@ -40,7 +41,7 @@ struct QLitDictEntry {
 #define QLIT_QBOOL(val) \
     { .type = QTYPE_QBOOL, .value.qbool = (val) }
 #define QLIT_QNUM_INT(val) \
-    { .type = QTYPE_QNUM, .value.qnum = (val) }
+    { .type = QTYPE_QNUM, .value.qnum = QNUM_VAL_INT(val) }
 #define QLIT_QSTR(val) \
     { .type = QTYPE_QSTRING, .value.qstr = (val) }
 #define QLIT_QDICT(val) \
diff --git a/qobject/qlit.c b/qobject/qlit.c
index 67126b25d5..b7af81cefb 100644
--- a/qobject/qlit.c
+++ b/qobject/qlit.c
@@ -71,7 +71,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:
-        return qnum_value_is_equal(&(QNumValue)QNUM_VAL_INT(lhs->value.qnum),
+        return qnum_value_is_equal(&lhs->value.qnum,
                                    &qobject_to(QNum, rhs)->value);
     case QTYPE_QSTRING:
         return (strcmp(lhs->value.qstr,
@@ -95,7 +95,7 @@ QObject *qobject_from_qlit(const QLitObject *qlit)
     case QTYPE_QNULL:
         return QOBJECT(qnull());
     case QTYPE_QNUM:
-        return QOBJECT(qnum_from_int(qlit->value.qnum));
+        return QOBJECT(qnum_from_value(qlit->value.qnum));
     case QTYPE_QSTRING:
         return QOBJECT(qstring_from_str(qlit->value.qstr));
     case QTYPE_QDICT: {
-- 
2.28.0

[PATCH v3 08/19] qlit: Move qlit_equal_qobject() reference values to array

[Eduardo Habkost]

Add an array of values to qlit_equal_qobject_test(), so we can
extend the test case to compare multiple literals, not just the
ones at the existing `qlit` and `qlit_foo` variables.

Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
This is a new patch added in v3 of this series.
---
 tests/check-qlit.c | 26 +++++++++++++++++++-------
 1 file changed, 19 insertions(+), 7 deletions(-)

diff --git a/tests/check-qlit.c b/tests/check-qlit.c
index 24ac21395c..b1cfbddb17 100644
--- a/tests/check-qlit.c
+++ b/tests/check-qlit.c
@@ -29,11 +29,6 @@ static QLitObject qlit = QLIT_QDICT(((QLitDictEntry[]) {
     { },
 }));
 
-static QLitObject qlit_foo = QLIT_QDICT(((QLitDictEntry[]) {
-    { "foo", QLIT_QNUM_INT(42) },
-    { },
-}));
-
 static QObject *make_qobject(void)
 {
     QDict *qdict = qdict_new();
@@ -53,16 +48,33 @@ static QObject *make_qobject(void)
 
 static void qlit_equal_qobject_test(void)
 {
+    /* Each entry in the values[] array should be different from the others */
+    QLitObject values[] = {
+        qlit,
+        QLIT_QDICT(((QLitDictEntry[]) {
+            { "foo", QLIT_QNUM_INT(42) },
+            { },
+        })),
+    };
+    int i;
     QObject *qobj = make_qobject();
 
     g_assert(qlit_equal_qobject(&qlit, qobj));
 
-    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_unref(qobj);
+
+    for (i = 0; i < ARRAY_SIZE(values); i++) {
+        int j;
+        QObject *o = qobject_from_qlit(&values[i]);
+        for (j = 0; j < ARRAY_SIZE(values); j++) {
+            g_assert(qlit_equal_qobject(&values[j], o) == (i == j));
+        }
+        qobject_unref(o);
+    }
+
 }
 
 static void qlit_equal_large_qnum_test(void)
-- 
2.28.0

Re: [PATCH v3 08/19] qlit: Move qlit_equal_qobject() reference values to array

[Markus Armbruster]

Eduardo Habkost <ehabkost@redhat.com> writes:

> Add an array of values to qlit_equal_qobject_test(), so we can
> extend the test case to compare multiple literals, not just the
> ones at the existing `qlit` and `qlit_foo` variables.
>
> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
> ---
> This is a new patch added in v3 of this series.
> ---
>  tests/check-qlit.c | 26 +++++++++++++++++++-------
>  1 file changed, 19 insertions(+), 7 deletions(-)
>
> diff --git a/tests/check-qlit.c b/tests/check-qlit.c
> index 24ac21395c..b1cfbddb17 100644
> --- a/tests/check-qlit.c
> +++ b/tests/check-qlit.c
> @@ -29,11 +29,6 @@ static QLitObject qlit = QLIT_QDICT(((QLitDictEntry[]) {
>      { },
>  }));
>  
> -static QLitObject qlit_foo = QLIT_QDICT(((QLitDictEntry[]) {
> -    { "foo", QLIT_QNUM_INT(42) },
> -    { },
> -}));
> -
>  static QObject *make_qobject(void)
>  {
>      QDict *qdict = qdict_new();
> @@ -53,16 +48,33 @@ static QObject *make_qobject(void)
>  
>  static void qlit_equal_qobject_test(void)
>  {
> +    /* Each entry in the values[] array should be different from the others */
> +    QLitObject values[] = {
> +        qlit,
> +        QLIT_QDICT(((QLitDictEntry[]) {
> +            { "foo", QLIT_QNUM_INT(42) },
> +            { },
> +        })),
> +    };
> +    int i;
>      QObject *qobj = make_qobject();
>  
>      g_assert(qlit_equal_qobject(&qlit, qobj));
>  
> -    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_unref(qobj);
> +
> +    for (i = 0; i < ARRAY_SIZE(values); i++) {
> +        int j;

I'd prefer to declare this one together with @i.

> +        QObject *o = qobject_from_qlit(&values[i]);

Blank line, if you don't mind.

> +        for (j = 0; j < ARRAY_SIZE(values); j++) {
> +            g_assert(qlit_equal_qobject(&values[j], o) == (i == j));
> +        }
> +        qobject_unref(o);
> +    }
> +
>  }
>  
>  static void qlit_equal_large_qnum_test(void)

Re: [PATCH v3 08/19] qlit: Move qlit_equal_qobject() reference values to array

[Eduardo Habkost]

On Tue, Nov 24, 2020 at 10:51:34AM +0100, Markus Armbruster wrote:
> Eduardo Habkost <ehabkost@redhat.com> writes:
> 
> > Add an array of values to qlit_equal_qobject_test(), so we can
> > extend the test case to compare multiple literals, not just the
> > ones at the existing `qlit` and `qlit_foo` variables.
> >
> > Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
> > ---
> > This is a new patch added in v3 of this series.
> > ---
> >  tests/check-qlit.c | 26 +++++++++++++++++++-------
> >  1 file changed, 19 insertions(+), 7 deletions(-)
> >
> > diff --git a/tests/check-qlit.c b/tests/check-qlit.c
> > index 24ac21395c..b1cfbddb17 100644
> > --- a/tests/check-qlit.c
> > +++ b/tests/check-qlit.c
> > @@ -29,11 +29,6 @@ static QLitObject qlit = QLIT_QDICT(((QLitDictEntry[]) {
> >      { },
> >  }));
> >  
> > -static QLitObject qlit_foo = QLIT_QDICT(((QLitDictEntry[]) {
> > -    { "foo", QLIT_QNUM_INT(42) },
> > -    { },
> > -}));
> > -
> >  static QObject *make_qobject(void)
> >  {
> >      QDict *qdict = qdict_new();
> > @@ -53,16 +48,33 @@ static QObject *make_qobject(void)
> >  
> >  static void qlit_equal_qobject_test(void)
> >  {
> > +    /* Each entry in the values[] array should be different from the others */
> > +    QLitObject values[] = {
> > +        qlit,
> > +        QLIT_QDICT(((QLitDictEntry[]) {
> > +            { "foo", QLIT_QNUM_INT(42) },
> > +            { },
> > +        })),
> > +    };
> > +    int i;
> >      QObject *qobj = make_qobject();
> >  
> >      g_assert(qlit_equal_qobject(&qlit, qobj));
> >  
> > -    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_unref(qobj);
> > +
> > +    for (i = 0; i < ARRAY_SIZE(values); i++) {
> > +        int j;
> 
> I'd prefer to declare this one together with @i.
> 
> > +        QObject *o = qobject_from_qlit(&values[i]);
> 
> Blank line, if you don't mind.

I will surely do it if there's a v4, but I hope you don't make me
submit v4 just to change these.

> 
> > +        for (j = 0; j < ARRAY_SIZE(values); j++) {
> > +            g_assert(qlit_equal_qobject(&values[j], o) == (i == j));
> > +        }
> > +        qobject_unref(o);
> > +    }
> > +
> >  }
> >  
> >  static void qlit_equal_large_qnum_test(void)

-- 
Eduardo

Re: [PATCH v3 08/19] qlit: Move qlit_equal_qobject() reference values to array

[Markus Armbruster]

Eduardo Habkost <ehabkost@redhat.com> writes:

> On Tue, Nov 24, 2020 at 10:51:34AM +0100, Markus Armbruster wrote:
>> Eduardo Habkost <ehabkost@redhat.com> writes:
>> 
>> > Add an array of values to qlit_equal_qobject_test(), so we can
>> > extend the test case to compare multiple literals, not just the
>> > ones at the existing `qlit` and `qlit_foo` variables.
>> >
>> > Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
>> > ---
>> > This is a new patch added in v3 of this series.
>> > ---
>> >  tests/check-qlit.c | 26 +++++++++++++++++++-------
>> >  1 file changed, 19 insertions(+), 7 deletions(-)
>> >
>> > diff --git a/tests/check-qlit.c b/tests/check-qlit.c
>> > index 24ac21395c..b1cfbddb17 100644
>> > --- a/tests/check-qlit.c
>> > +++ b/tests/check-qlit.c
>> > @@ -29,11 +29,6 @@ static QLitObject qlit = QLIT_QDICT(((QLitDictEntry[]) {
>> >      { },
>> >  }));
>> >  
>> > -static QLitObject qlit_foo = QLIT_QDICT(((QLitDictEntry[]) {
>> > -    { "foo", QLIT_QNUM_INT(42) },
>> > -    { },
>> > -}));
>> > -
>> >  static QObject *make_qobject(void)
>> >  {
>> >      QDict *qdict = qdict_new();
>> > @@ -53,16 +48,33 @@ static QObject *make_qobject(void)
>> >  
>> >  static void qlit_equal_qobject_test(void)
>> >  {
>> > +    /* Each entry in the values[] array should be different from the others */
>> > +    QLitObject values[] = {
>> > +        qlit,
>> > +        QLIT_QDICT(((QLitDictEntry[]) {
>> > +            { "foo", QLIT_QNUM_INT(42) },
>> > +            { },
>> > +        })),
>> > +    };
>> > +    int i;
>> >      QObject *qobj = make_qobject();
>> >  
>> >      g_assert(qlit_equal_qobject(&qlit, qobj));
>> >  
>> > -    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_unref(qobj);
>> > +
>> > +    for (i = 0; i < ARRAY_SIZE(values); i++) {
>> > +        int j;
>> 
>> I'd prefer to declare this one together with @i.
>> 
>> > +        QObject *o = qobject_from_qlit(&values[i]);
>> 
>> Blank line, if you don't mind.
>
> I will surely do it if there's a v4, but I hope you don't make me
> submit v4 just to change these.

That would be a waste of your time and mine :)

If we decide to take the patches through my tree, I can tweak this in my
tree.

If somebody else takes it, and prefers not to tweak, I can tweak on top
if I care.

>
>> 
>> > +        for (j = 0; j < ARRAY_SIZE(values); j++) {
>> > +            g_assert(qlit_equal_qobject(&values[j], o) == (i == j));
>> > +        }
>> > +        qobject_unref(o);
>> > +    }
>> > +
>> >  }
>> >  
>> >  static void qlit_equal_large_qnum_test(void)

[PATCH v3 09/19] qlit: Add more test literals to qlit_equal_qobject() test case

[Eduardo Habkost]

Add a few examples of each qlit type, to make sure each one
compare as equal to itself, but not equal to the other values.

Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
This is a new patch added in v3 of this series.
---
 tests/check-qlit.c | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/tests/check-qlit.c b/tests/check-qlit.c
index b1cfbddb17..5a9260b93f 100644
--- a/tests/check-qlit.c
+++ b/tests/check-qlit.c
@@ -50,11 +50,27 @@ static void qlit_equal_qobject_test(void)
 {
     /* Each entry in the values[] array should be different from the others */
     QLitObject values[] = {
+        QLIT_QNULL,
+        QLIT_QBOOL(false),
+        QLIT_QBOOL(true),
+        QLIT_QNUM_INT(-1),
+        QLIT_QNUM_INT(0),
+        QLIT_QNUM_INT(1),
+        QLIT_QNUM_INT(INT64_MIN),
+        QLIT_QNUM_INT(INT64_MAX),
+        QLIT_QSTR(""),
+        QLIT_QSTR("foo"),
         qlit,
         QLIT_QDICT(((QLitDictEntry[]) {
             { "foo", QLIT_QNUM_INT(42) },
             { },
         })),
+        QLIT_QLIST(((QLitObject[]){
+            QLIT_QNUM_INT(-1),
+            QLIT_QNUM_INT(0),
+            QLIT_QNUM_INT(1),
+            { },
+        })),
     };
     int i;
     QObject *qobj = make_qobject();
-- 
2.28.0

[PATCH v3 10/19] qlit: Support all types of QNums

[Eduardo Habkost]

Add two new macros to support other types of QNums:
QLIT_QNUM_UINT, and QLIT_QNUM_DOUBLE, and include them
in the qlit_equal_qobject_test() test case.

Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
Changes v2 -> v3:
* QLIT_QNUM macro doesn't exist anymore
* Addition of the QNumValue field to QLitObject is
  now in a separate patch ("qlit: Use QNumValue to represent QNums")
* check-qjson test case changes dropped.
  Instead, I'm only extending the qlit_equal_qobject_test() test
  case.

Changes v1 -> v2:
* Coding style fix at qlit_equal_qobject()
---
 include/qapi/qmp/qlit.h | 4 ++++
 tests/check-qlit.c      | 5 +++++
 2 files changed, 9 insertions(+)

diff --git a/include/qapi/qmp/qlit.h b/include/qapi/qmp/qlit.h
index a240cdd299..a2881b7f42 100644
--- a/include/qapi/qmp/qlit.h
+++ b/include/qapi/qmp/qlit.h
@@ -42,6 +42,10 @@ struct QLitDictEntry {
     { .type = QTYPE_QBOOL, .value.qbool = (val) }
 #define QLIT_QNUM_INT(val) \
     { .type = QTYPE_QNUM, .value.qnum = QNUM_VAL_INT(val) }
+#define QLIT_QNUM_UINT(val) \
+    { .type = QTYPE_QNUM, .value.qnum = QNUM_VAL_UINT(val) }
+#define QLIT_QNUM_DOUBLE(val) \
+    { .type = QTYPE_QNUM, .value.qnum = QNUM_VAL_DOUBLE(val) }
 #define QLIT_QSTR(val) \
     { .type = QTYPE_QSTRING, .value.qstr = (val) }
 #define QLIT_QDICT(val) \
diff --git a/tests/check-qlit.c b/tests/check-qlit.c
index 5a9260b93f..31e90f8965 100644
--- a/tests/check-qlit.c
+++ b/tests/check-qlit.c
@@ -58,6 +58,11 @@ static void qlit_equal_qobject_test(void)
         QLIT_QNUM_INT(1),
         QLIT_QNUM_INT(INT64_MIN),
         QLIT_QNUM_INT(INT64_MAX),
+        QLIT_QNUM_UINT(UINT64_MAX),
+        /* Larger than UINT64_MAX: */
+        QLIT_QNUM_DOUBLE(18446744073709552e3),
+        /* Smaller than INT64_MIN: */
+        QLIT_QNUM_DOUBLE(-92233720368547758e2),
         QLIT_QSTR(""),
         QLIT_QSTR("foo"),
         qlit,
-- 
2.28.0

Re: [PATCH v3 10/19] qlit: Support all types of QNums

[Markus Armbruster]

Eduardo Habkost <ehabkost@redhat.com> writes:

> Add two new macros to support other types of QNums:
> QLIT_QNUM_UINT, and QLIT_QNUM_DOUBLE, and include them
> in the qlit_equal_qobject_test() test case.
>
> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
> ---
> Changes v2 -> v3:
> * QLIT_QNUM macro doesn't exist anymore
> * Addition of the QNumValue field to QLitObject is
>   now in a separate patch ("qlit: Use QNumValue to represent QNums")
> * check-qjson test case changes dropped.
>   Instead, I'm only extending the qlit_equal_qobject_test() test
>   case.
>
> Changes v1 -> v2:
> * Coding style fix at qlit_equal_qobject()
> ---
>  include/qapi/qmp/qlit.h | 4 ++++
>  tests/check-qlit.c      | 5 +++++
>  2 files changed, 9 insertions(+)
>
> diff --git a/include/qapi/qmp/qlit.h b/include/qapi/qmp/qlit.h
> index a240cdd299..a2881b7f42 100644
> --- a/include/qapi/qmp/qlit.h
> +++ b/include/qapi/qmp/qlit.h
> @@ -42,6 +42,10 @@ struct QLitDictEntry {
>      { .type = QTYPE_QBOOL, .value.qbool = (val) }
>  #define QLIT_QNUM_INT(val) \
>      { .type = QTYPE_QNUM, .value.qnum = QNUM_VAL_INT(val) }
> +#define QLIT_QNUM_UINT(val) \
> +    { .type = QTYPE_QNUM, .value.qnum = QNUM_VAL_UINT(val) }
> +#define QLIT_QNUM_DOUBLE(val) \
> +    { .type = QTYPE_QNUM, .value.qnum = QNUM_VAL_DOUBLE(val) }
>  #define QLIT_QSTR(val) \
>      { .type = QTYPE_QSTRING, .value.qstr = (val) }
>  #define QLIT_QDICT(val) \
> diff --git a/tests/check-qlit.c b/tests/check-qlit.c
> index 5a9260b93f..31e90f8965 100644
> --- a/tests/check-qlit.c
> +++ b/tests/check-qlit.c
> @@ -58,6 +58,11 @@ static void qlit_equal_qobject_test(void)
>          QLIT_QNUM_INT(1),
>          QLIT_QNUM_INT(INT64_MIN),
>          QLIT_QNUM_INT(INT64_MAX),
> +        QLIT_QNUM_UINT(UINT64_MAX),
> +        /* Larger than UINT64_MAX: */
> +        QLIT_QNUM_DOUBLE(18446744073709552e3),
> +        /* Smaller than INT64_MIN: */
> +        QLIT_QNUM_DOUBLE(-92233720368547758e2),

Why "larger than UINT64_MAX" and "smaller than INT64_MIN"?

>          QLIT_QSTR(""),
>          QLIT_QSTR("foo"),
>          qlit,

Re: [PATCH v3 10/19] qlit: Support all types of QNums

[Paolo Bonzini]

On 24/11/20 10:55, Markus Armbruster wrote:
>> +        /* Larger than UINT64_MAX: */
>> +        QLIT_QNUM_DOUBLE(18446744073709552e3),
>> +        /* Smaller than INT64_MIN: */
>> +        QLIT_QNUM_DOUBLE(-92233720368547758e2),
> Why "larger than UINT64_MAX" and "smaller than INT64_MIN"?
> 

I guess the point is to test values that are only representable as a 
double, so (double)((uint64_t)INT64_MAX+1) wouldn't be very useful for 
that: as the expression shows, it would not be a QNUM_VAL_INT but it 
would be representable as QNUM_VAL_UINT.

So these are the cases that matter the most, even though -1, 0 and 
INT64_MAX+1 could be nice to have.

Paolo

Re: [PATCH v3 10/19] qlit: Support all types of QNums

[Markus Armbruster]

Paolo Bonzini <pbonzini@redhat.com> writes:

> On 24/11/20 10:55, Markus Armbruster wrote:
>>> +        /* Larger than UINT64_MAX: */
>>> +        QLIT_QNUM_DOUBLE(18446744073709552e3),
>>> +        /* Smaller than INT64_MIN: */
>>> +        QLIT_QNUM_DOUBLE(-92233720368547758e2),
>> Why "larger than UINT64_MAX" and "smaller than INT64_MIN"?
>> 
>
> I guess the point is to test values that are only representable as a
> double, so (double)((uint64_t)INT64_MAX+1) wouldn't be very useful for 
> that: as the expression shows, it would not be a QNUM_VAL_INT but it
> would be representable as QNUM_VAL_UINT.
>
> So these are the cases that matter the most, even though -1, 0 and
> INT64_MAX+1 could be nice to have.

qnum_is_equal()'s contract:

 * Doubles are never considered equal to integers.

Re: [PATCH v3 10/19] qlit: Support all types of QNums

[Eduardo Habkost]

On Tue, Nov 24, 2020 at 01:22:02PM +0100, Markus Armbruster wrote:
> Paolo Bonzini <pbonzini@redhat.com> writes:
> 
> > On 24/11/20 10:55, Markus Armbruster wrote:
> >>> +        /* Larger than UINT64_MAX: */
> >>> +        QLIT_QNUM_DOUBLE(18446744073709552e3),
> >>> +        /* Smaller than INT64_MIN: */
> >>> +        QLIT_QNUM_DOUBLE(-92233720368547758e2),
> >> Why "larger than UINT64_MAX" and "smaller than INT64_MIN"?
> >> 
> >
> > I guess the point is to test values that are only representable as a
> > double, so (double)((uint64_t)INT64_MAX+1) wouldn't be very useful for 
> > that: as the expression shows, it would not be a QNUM_VAL_INT but it
> > would be representable as QNUM_VAL_UINT.
> >
> > So these are the cases that matter the most, even though -1, 0 and
> > INT64_MAX+1 could be nice to have.
> 
> qnum_is_equal()'s contract:
> 
>  * Doubles are never considered equal to integers.

If that's part of the contract, it would be OK to include
0.0, -1.0, 1.0, INT64_MAX+1 in the list.  I incorrectly assumed
  qnum_is_equal(qnum_from_int(0), qnum_from_double(0.0))
was undefined.

However, if we really care about test coverage of
qnum_is_equal(), we probably should be extending check-qnum.c,
not check-qlit.c.

-- 
Eduardo

[PATCH v3 11/19] qom: field_prop_set_default_value() helper

[Eduardo Habkost]

Move code that sets the property default value to a separate
function, to reduce duplication and make refactoring easier.

Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
This is a new patch added in v3 of this series.
Hopefully, this will make the series easier to review.

The field_prop_set_default_value() was added in v2 at
"qom: Use qlit to represent property defaults".
---
 qom/field-property.c | 24 ++++++++++++++++--------
 1 file changed, 16 insertions(+), 8 deletions(-)

diff --git a/qom/field-property.c b/qom/field-property.c
index cb729626ce..6a0df7c6ea 100644
--- a/qom/field-property.c
+++ b/qom/field-property.c
@@ -62,6 +62,17 @@ static void static_prop_release_dynamic_prop(Object *obj, const char *name,
     g_free(prop);
 }
 
+static void field_prop_set_default_value(ObjectProperty *op,
+                                         Property *prop)
+{
+    if (!prop->set_default) {
+        return;
+    }
+
+    assert(prop->info->set_default_value);
+    prop->info->set_default_value(op, prop);
+}
+
 ObjectProperty *
 object_property_add_field(Object *obj, const char *name,
                           Property *prop,
@@ -83,11 +94,9 @@ object_property_add_field(Object *obj, const char *name,
     object_property_set_description(obj, name,
                                     newprop->info->description);
 
-    if (newprop->set_default) {
-        newprop->info->set_default_value(op, newprop);
-        if (op->init) {
-            op->init(obj, op);
-        }
+    field_prop_set_default_value(op, newprop);
+    if (op->init) {
+        op->init(obj, op);
     }
 
     op->allow_set = allow_set;
@@ -113,9 +122,8 @@ object_class_property_add_field_static(ObjectClass *oc, const char *name,
                                        prop->info->release,
                                        prop);
     }
-    if (prop->set_default) {
-        prop->info->set_default_value(op, prop);
-    }
+
+    field_prop_set_default_value(op, prop);
     if (prop->info->description) {
         object_class_property_set_description(oc, name,
                                               prop->info->description);
-- 
2.28.0

Re: [PATCH v3 11/19] qom: field_prop_set_default_value() helper

[Markus Armbruster]

Eduardo Habkost <ehabkost@redhat.com> writes:

> Move code that sets the property default value to a separate
> function, to reduce duplication and make refactoring easier.
>
> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
> ---
> This is a new patch added in v3 of this series.
> Hopefully, this will make the series easier to review.
>
> The field_prop_set_default_value() was added in v2 at
> "qom: Use qlit to represent property defaults".
> ---
>  qom/field-property.c | 24 ++++++++++++++++--------
>  1 file changed, 16 insertions(+), 8 deletions(-)
>
> diff --git a/qom/field-property.c b/qom/field-property.c
> index cb729626ce..6a0df7c6ea 100644
> --- a/qom/field-property.c
> +++ b/qom/field-property.c
> @@ -62,6 +62,17 @@ static void static_prop_release_dynamic_prop(Object *obj, const char *name,
>      g_free(prop);
>  }
>  
> +static void field_prop_set_default_value(ObjectProperty *op,
> +                                         Property *prop)
> +{
> +    if (!prop->set_default) {
> +        return;
> +    }
> +
> +    assert(prop->info->set_default_value);
> +    prop->info->set_default_value(op, prop);
> +}
> +
>  ObjectProperty *
>  object_property_add_field(Object *obj, const char *name,
>                            Property *prop,
> @@ -83,11 +94,9 @@ object_property_add_field(Object *obj, const char *name,
>      object_property_set_description(obj, name,
>                                      newprop->info->description);
>  
> -    if (newprop->set_default) {
> -        newprop->info->set_default_value(op, newprop);
> -        if (op->init) {
> -            op->init(obj, op);
> -        }
> +    field_prop_set_default_value(op, newprop);
> +    if (op->init) {
> +        op->init(obj, op);
>      }

op->init() is now called even when !newprop->set_default.  Why is that
okay?

>  
>      op->allow_set = allow_set;
> @@ -113,9 +122,8 @@ object_class_property_add_field_static(ObjectClass *oc, const char *name,
>                                         prop->info->release,
>                                         prop);
>      }
> -    if (prop->set_default) {
> -        prop->info->set_default_value(op, prop);
> -    }
> +
> +    field_prop_set_default_value(op, prop);
>      if (prop->info->description) {
>          object_class_property_set_description(oc, name,
>                                                prop->info->description);

Re: [PATCH v3 11/19] qom: field_prop_set_default_value() helper

[Eduardo Habkost]

On Tue, Nov 24, 2020 at 10:58:27AM +0100, Markus Armbruster wrote:
> Eduardo Habkost <ehabkost@redhat.com> writes:
> 
> > Move code that sets the property default value to a separate
> > function, to reduce duplication and make refactoring easier.
> >
> > Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
> > ---
> > This is a new patch added in v3 of this series.
> > Hopefully, this will make the series easier to review.
> >
> > The field_prop_set_default_value() was added in v2 at
> > "qom: Use qlit to represent property defaults".
> > ---
> >  qom/field-property.c | 24 ++++++++++++++++--------
> >  1 file changed, 16 insertions(+), 8 deletions(-)
> >
> > diff --git a/qom/field-property.c b/qom/field-property.c
> > index cb729626ce..6a0df7c6ea 100644
> > --- a/qom/field-property.c
> > +++ b/qom/field-property.c
> > @@ -62,6 +62,17 @@ static void static_prop_release_dynamic_prop(Object *obj, const char *name,
> >      g_free(prop);
> >  }
> >  
> > +static void field_prop_set_default_value(ObjectProperty *op,
> > +                                         Property *prop)
> > +{
> > +    if (!prop->set_default) {
> > +        return;
> > +    }
> > +
> > +    assert(prop->info->set_default_value);
> > +    prop->info->set_default_value(op, prop);
> > +}
> > +
> >  ObjectProperty *
> >  object_property_add_field(Object *obj, const char *name,
> >                            Property *prop,
> > @@ -83,11 +94,9 @@ object_property_add_field(Object *obj, const char *name,
> >      object_property_set_description(obj, name,
> >                                      newprop->info->description);
> >  
> > -    if (newprop->set_default) {
> > -        newprop->info->set_default_value(op, newprop);
> > -        if (op->init) {
> > -            op->init(obj, op);
> > -        }
> > +    field_prop_set_default_value(op, newprop);
> > +    if (op->init) {
> > +        op->init(obj, op);
> >      }
> 
> op->init() is now called even when !newprop->set_default.  Why is that
> okay?

op->init() will be NULL if object_property_set_default() was not
called.

It's subtle, and worth mentioning in the commit message.  I think
we should encapsulate op->init() behind a
object_property_reset_to_default_value() function.

> 
> >  
> >      op->allow_set = allow_set;
> > @@ -113,9 +122,8 @@ object_class_property_add_field_static(ObjectClass *oc, const char *name,
> >                                         prop->info->release,
> >                                         prop);
> >      }
> > -    if (prop->set_default) {
> > -        prop->info->set_default_value(op, prop);
> > -    }
> > +
> > +    field_prop_set_default_value(op, prop);
> >      if (prop->info->description) {
> >          object_class_property_set_description(oc, name,
> >                                                prop->info->description);

-- 
Eduardo

[PATCH v3 12/19] qom: Replace defval value in Property with QLitObject

[Eduardo Habkost]

QLitObject is capable of representing a wider range of values,
and it will allow us to simplify a lot of the existing code
setting property defaults, later.  Replace the bool and union
fields with QLitObject.

In short, this is just a direct translation from:
  prop->set_default
to
  prop->defval.type != QTYPE_NONE

from
  prop->defval.i
to
  qnum_get_int(qobject_to(QNum, prop->defval)

and from
  prop->defval.u
to
  qnum_get_uint(qobject_to(QNum, prop->defval)

Note that this doesn't replace any of the default property
setters (yet), but just make them safer.  Now the actual type of
.defval is validated before it is used.

Also note that set_default_uuid_auto() is a bit weird: it ignores
.defval completely.  This patch keeps the existing behavior, and
set_default_uuid_auto() weirdness will be addressed later.

Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
This is a new patch added in v3 of this series.

I am splitting the changes and making them in smaller steps to
make them easier to understand and review.  With this, I intend
to demonstrate that the conversion from bool+union to QLitObject
is an improvement even if the removal of custom .set_defaul_value
functions isn't 100% finished yet.
---
 include/hw/qdev-properties-system.h   |  7 ++++++-
 include/qom/field-property-internal.h |  9 ++++++---
 include/qom/field-property.h          | 27 ++++++++++-----------------
 include/qom/property-types.h          | 18 ++++++------------
 hw/core/qdev-properties-system.c      |  3 ++-
 qom/field-property.c                  |  8 ++++++--
 qom/property-types.c                  | 26 ++++++++++++++++++--------
 7 files changed, 54 insertions(+), 44 deletions(-)

diff --git a/include/hw/qdev-properties-system.h b/include/hw/qdev-properties-system.h
index 0ac327ae60..6221da704e 100644
--- a/include/hw/qdev-properties-system.h
+++ b/include/hw/qdev-properties-system.h
@@ -65,7 +65,12 @@ extern const PropertyInfo qdev_prop_pcie_link_width;
 
 #define DEFINE_PROP_UUID(_name, _state, _field) \
     DEFINE_PROP(_name, _state, _field, qdev_prop_uuid, QemuUUID, \
-                .set_default = true)
+                /*                                               \
+                 * Note that set_default_uuid_auto() currently   \
+                 * ignores the actual value value of .defval,    \
+                 * we just need it to not be not QTYPE_NONE      \
+                 */                                              \
+                .defval = QLIT_QNULL)
 
 #define DEFINE_PROP_AUDIODEV(_n, _s, _f) \
     DEFINE_PROP(_n, _s, _f, qdev_prop_audiodev, QEMUSoundCard)
diff --git a/include/qom/field-property-internal.h b/include/qom/field-property-internal.h
index a7b7e2b69d..7ed0d8d160 100644
--- a/include/qom/field-property-internal.h
+++ b/include/qom/field-property-internal.h
@@ -14,11 +14,14 @@ void field_prop_set_enum(Object *obj, Visitor *v, const char *name,
                          void *opaque, Error **errp);
 
 void field_prop_set_default_value_enum(ObjectProperty *op,
-                                       const Property *prop);
+                                       const Property *prop,
+                                       const QObject *defval);
 void field_prop_set_default_value_int(ObjectProperty *op,
-                                      const Property *prop);
+                                      const Property *prop,
+                                       const QObject *defval);
 void field_prop_set_default_value_uint(ObjectProperty *op,
-                                       const Property *prop);
+                                       const Property *prop,
+                                       const QObject *defval);
 
 void field_prop_get_int32(Object *obj, Visitor *v, const char *name,
                           void *opaque, Error **errp);
diff --git a/include/qom/field-property.h b/include/qom/field-property.h
index 0cb1fe2217..951cec2fb0 100644
--- a/include/qom/field-property.h
+++ b/include/qom/field-property.h
@@ -6,6 +6,7 @@
 
 #include "qom/object.h"
 #include "qapi/util.h"
+#include "qapi/qmp/qlit.h"
 
 /**
  * struct Property: definition of a field property
@@ -27,21 +28,8 @@ struct Property {
     const PropertyInfo *info;
     ptrdiff_t    offset;
     uint8_t      bitnr;
-    /**
-     * @set_default: true if the default value should be set from @defval,
-     *    in which case @info->set_default_value must not be NULL
-     *    (if false then no default value is set by the property system
-     *     and the field retains whatever value it was given by instance_init).
-     */
-    bool         set_default;
-    /**
-     * @defval: default value for the property. This is used only if @set_default
-     *     is true.
-     */
-    union {
-        int64_t i;
-        uint64_t u;
-    } defval;
+    /** @defval: If not QTYPE_NONE, the default value for the property */
+    QLitObject defval;
     /* private: */
     int          arrayoffset;
     const PropertyInfo *arrayinfo;
@@ -61,8 +49,13 @@ struct PropertyInfo {
     const QEnumLookup *enum_table;
     /** @print: String formatting function, for the human monitor */
     int (*print)(Object *obj, Property *prop, char *dest, size_t len);
-    /** @set_default_value: Callback for initializing the default value */
-    void (*set_default_value)(ObjectProperty *op, const Property *prop);
+    /**
+     * @set_default_value: Callback for initializing the default value
+     *
+     * @defval is a weak reference.
+     */
+    void (*set_default_value)(ObjectProperty *op, const Property *prop,
+                              const QObject *defval);
     /** @create: Optional callback for creation of property */
     ObjectProperty *(*create)(ObjectClass *oc, const char *name,
                               Property *prop);
diff --git a/include/qom/property-types.h b/include/qom/property-types.h
index 3132ddafd9..46e691dab4 100644
--- a/include/qom/property-types.h
+++ b/include/qom/property-types.h
@@ -25,34 +25,29 @@ extern const PropertyInfo prop_info_link;
 
 #define PROP_SIGNED(_state, _field, _defval, _prop, _type, ...) \
     FIELD_PROP(_state, _field, _prop, _type,                    \
-               .set_default = true,                             \
-               .defval.i    = (_type)_defval,                   \
+               .defval = QLIT_QNUM_INT((_type)_defval),                \
                __VA_ARGS__)
 
 #define PROP_UNSIGNED(_state, _field, _defval, _prop, _type, ...) \
     FIELD_PROP(_state, _field, _prop, _type,                    \
-               .set_default = true,                             \
-               .defval.u  = (_type)_defval,                     \
+               .defval = QLIT_QNUM_UINT((_type)_defval),               \
                __VA_ARGS__)
 
 #define PROP_BIT(_state, _field, _bit, _defval, ...) \
     FIELD_PROP(_state, _field, prop_info_bit, uint32_t,         \
                .bitnr       = (_bit),                           \
-               .set_default = true,                             \
-               .defval.u    = (bool)_defval,                    \
+               .defval = QLIT_QBOOL(_defval),                   \
                __VA_ARGS__)
 
 #define PROP_BIT64(_state, _field, _bit, _defval, ...) \
     FIELD_PROP(_state, _field, prop_info_bit64, uint64_t,       \
                .bitnr    = (_bit),                              \
-               .set_default = true,                             \
-               .defval.u  = (bool)_defval,                      \
+               .defval = QLIT_QBOOL(_defval),                   \
                __VA_ARGS__)
 
 #define PROP_BOOL(_state, _field, _defval, ...) \
     FIELD_PROP(_state, _field, prop_info_bool, bool,            \
-               .set_default = true,                             \
-               .defval.u    = (bool)_defval,                    \
+               .defval = QLIT_QBOOL(_defval),                   \
                __VA_ARGS__)
 
 #define PROP_LINK(_state, _field, _type, _ptr_type, ...) \
@@ -131,8 +126,7 @@ extern const PropertyInfo prop_info_link;
                           _arrayfield, _arrayprop, _arraytype) \
     DEFINE_PROP((PROP_ARRAY_LEN_PREFIX _name),                 \
                 _state, _field, prop_info_arraylen, uint32_t,  \
-                .set_default = true,                           \
-                .defval.u = 0,                                 \
+                .defval = QLIT_QNUM_UINT(0),                   \
                 .arrayinfo = &(_arrayprop),                    \
                 .arrayfieldsize = sizeof(_arraytype),          \
                 .arrayoffset = offsetof(_state, _arrayfield))
diff --git a/hw/core/qdev-properties-system.c b/hw/core/qdev-properties-system.c
index 8da68f076c..217e041152 100644
--- a/hw/core/qdev-properties-system.c
+++ b/hw/core/qdev-properties-system.c
@@ -1080,7 +1080,8 @@ static void set_uuid(Object *obj, Visitor *v, const char *name, void *opaque,
     g_free(str);
 }
 
-static void set_default_uuid_auto(ObjectProperty *op, const Property *prop)
+static void set_default_uuid_auto(ObjectProperty *op, const Property *prop,
+                                  const QObject *defval)
 {
     object_property_set_default_str(op, UUID_VALUE_AUTO);
 }
diff --git a/qom/field-property.c b/qom/field-property.c
index 6a0df7c6ea..593ffb53e9 100644
--- a/qom/field-property.c
+++ b/qom/field-property.c
@@ -65,12 +65,16 @@ static void static_prop_release_dynamic_prop(Object *obj, const char *name,
 static void field_prop_set_default_value(ObjectProperty *op,
                                          Property *prop)
 {
-    if (!prop->set_default) {
+    QObject *defval;
+
+    if (prop->defval.type == QTYPE_NONE) {
         return;
     }
 
+    defval = qobject_from_qlit(&prop->defval);
     assert(prop->info->set_default_value);
-    prop->info->set_default_value(op, prop);
+    prop->info->set_default_value(op, prop, defval);
+    qobject_unref(defval);
 }
 
 ObjectProperty *
diff --git a/qom/property-types.c b/qom/property-types.c
index 4b3fe15b6a..0fc24b3687 100644
--- a/qom/property-types.c
+++ b/qom/property-types.c
@@ -6,6 +6,8 @@
 #include "qapi/visitor.h"
 #include "qapi/error.h"
 #include "qemu/uuid.h"
+#include "qapi/qmp/qnum.h"
+#include "qapi/qmp/qbool.h"
 
 void field_prop_get_enum(Object *obj, Visitor *v, const char *name,
                          void *opaque, Error **errp)
@@ -26,10 +28,12 @@ void field_prop_set_enum(Object *obj, Visitor *v, const char *name,
 }
 
 void field_prop_set_default_value_enum(ObjectProperty *op,
-                                       const Property *prop)
+                                       const Property *prop,
+                                       const QObject *defval)
 {
+    QNum *qn = qobject_to(QNum, defval);
     object_property_set_default_str(op,
-        qapi_enum_lookup(prop->info->enum_table, prop->defval.i));
+        qapi_enum_lookup(prop->info->enum_table, qnum_get_int(qn)));
 }
 
 const PropertyInfo prop_info_enum = {
@@ -80,9 +84,11 @@ static void prop_set_bit(Object *obj, Visitor *v, const char *name,
     bit_prop_set(obj, prop, value);
 }
 
-static void set_default_value_bool(ObjectProperty *op, const Property *prop)
+static void set_default_value_bool(ObjectProperty *op, const Property *prop,
+                                   const QObject *defval)
 {
-    object_property_set_default_bool(op, prop->defval.u);
+    QBool *qb = qobject_to(QBool, defval);
+    object_property_set_default_bool(op, qbool_get_bool(qb));
 }
 
 const PropertyInfo prop_info_bit = {
@@ -190,15 +196,19 @@ static void set_uint8(Object *obj, Visitor *v, const char *name, void *opaque,
 }
 
 void field_prop_set_default_value_int(ObjectProperty *op,
-                                      const Property *prop)
+                                      const Property *prop,
+                                      const QObject *defval)
 {
-    object_property_set_default_int(op, prop->defval.i);
+    QNum *qn = qobject_to(QNum, defval);
+    object_property_set_default_int(op, qnum_get_int(qn));
 }
 
 void field_prop_set_default_value_uint(ObjectProperty *op,
-                                       const Property *prop)
+                                       const Property *prop,
+                                       const QObject *defval)
 {
-    object_property_set_default_uint(op, prop->defval.u);
+    QNum *qn = qobject_to(QNum, defval);
+    object_property_set_default_uint(op, qnum_get_uint(qn));
 }
 
 const PropertyInfo prop_info_uint8 = {
-- 
2.28.0

[PATCH v3 13/19] qom: Fix documentation of UUID property type

[Eduardo Habkost]

On some cases, the documentation for UUID properties is lying:
properties defined using DEFINE_PROP_UUID_NODEFAULT are not set
to "auto" by default.  It's better to omit this instead of
providing incorrect information.

Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
This is a new patch addeed in v3 of this series.
---
 hw/core/qdev-properties-system.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/core/qdev-properties-system.c b/hw/core/qdev-properties-system.c
index 217e041152..6071f672a4 100644
--- a/hw/core/qdev-properties-system.c
+++ b/hw/core/qdev-properties-system.c
@@ -1089,7 +1089,7 @@ static void set_default_uuid_auto(ObjectProperty *op, const Property *prop,
 const PropertyInfo qdev_prop_uuid = {
     .name  = "str",
     .description = "UUID (aka GUID) or \"" UUID_VALUE_AUTO
-        "\" for random value (default)",
+        "\" for random value",
     .get   = get_uuid,
     .set   = set_uuid,
     .set_default_value = set_default_uuid_auto,
-- 
2.28.0

[PATCH v3 14/19] qom: Don't ignore defval on UUID property

[Eduardo Habkost]

UUID properties were weird because the value of .defval was
completely ignored.  Fix this by setting the default value to
QLIT_QSTR("auto") on the default case, and actually using .defval
inside .set_default_value.

Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
This is a new patch added in v3 of this series.

This is similar (but not completely the same) to changes that
were submitted as part of
"[PATCH v2 8/8] qom: Use qlit to represent property defaults".
---
 include/hw/qdev-properties-system.h |  9 +++------
 hw/core/qdev-properties-system.c    | 10 +++++-----
 2 files changed, 8 insertions(+), 11 deletions(-)

diff --git a/include/hw/qdev-properties-system.h b/include/hw/qdev-properties-system.h
index 6221da704e..834ca84904 100644
--- a/include/hw/qdev-properties-system.h
+++ b/include/hw/qdev-properties-system.h
@@ -63,14 +63,11 @@ extern const PropertyInfo qdev_prop_pcie_link_width;
     DEFINE_PROP_SIGNED(_n, _s, _f, _d, qdev_prop_pcie_link_width, \
                         PCIExpLinkWidth)
 
+#define UUID_VALUE_AUTO        "auto"
+
 #define DEFINE_PROP_UUID(_name, _state, _field) \
     DEFINE_PROP(_name, _state, _field, qdev_prop_uuid, QemuUUID, \
-                /*                                               \
-                 * Note that set_default_uuid_auto() currently   \
-                 * ignores the actual value value of .defval,    \
-                 * we just need it to not be not QTYPE_NONE      \
-                 */                                              \
-                .defval = QLIT_QNULL)
+                .defval = QLIT_QSTR(UUID_VALUE_AUTO))
 
 #define DEFINE_PROP_AUDIODEV(_n, _s, _f) \
     DEFINE_PROP(_n, _s, _f, qdev_prop_audiodev, QEMUSoundCard)
diff --git a/hw/core/qdev-properties-system.c b/hw/core/qdev-properties-system.c
index 6071f672a4..117c540254 100644
--- a/hw/core/qdev-properties-system.c
+++ b/hw/core/qdev-properties-system.c
@@ -15,6 +15,7 @@
 #include "hw/qdev-properties-system.h"
 #include "qapi/error.h"
 #include "qapi/visitor.h"
+#include "qapi/qmp/qstring.h"
 #include "qapi/qapi-types-block.h"
 #include "qapi/qapi-types-machine.h"
 #include "qapi/qapi-types-migration.h"
@@ -1059,8 +1060,6 @@ static void get_uuid(Object *obj, Visitor *v, const char *name, void *opaque,
     visit_type_str(v, name, &p, errp);
 }
 
-#define UUID_VALUE_AUTO        "auto"
-
 static void set_uuid(Object *obj, Visitor *v, const char *name, void *opaque,
                     Error **errp)
 {
@@ -1080,10 +1079,11 @@ static void set_uuid(Object *obj, Visitor *v, const char *name, void *opaque,
     g_free(str);
 }
 
-static void set_default_uuid_auto(ObjectProperty *op, const Property *prop,
+static void set_default_uuid(ObjectProperty *op, const Property *prop,
                                   const QObject *defval)
 {
-    object_property_set_default_str(op, UUID_VALUE_AUTO);
+    QString *qs = qobject_to(QString, defval);
+    object_property_set_default_str(op, qstring_get_str(qs));
 }
 
 const PropertyInfo qdev_prop_uuid = {
@@ -1092,5 +1092,5 @@ const PropertyInfo qdev_prop_uuid = {
         "\" for random value",
     .get   = get_uuid,
     .set   = set_uuid,
-    .set_default_value = set_default_uuid_auto,
+    .set_default_value = set_default_uuid,
 };
-- 
2.28.0

[PATCH v3 15/19] qom: Make object_property_set_default() public

[Eduardo Habkost]

The function will be used outside qom/object.c, to simplify the
field property code that sets the property default value.

Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
 include/qom/object.h | 11 +++++++++++
 qom/object.c         |  2 +-
 2 files changed, 12 insertions(+), 1 deletion(-)

diff --git a/include/qom/object.h b/include/qom/object.h
index 2ab124b8f0..4234cc9b66 100644
--- a/include/qom/object.h
+++ b/include/qom/object.h
@@ -1090,6 +1090,17 @@ ObjectProperty *object_class_property_add(ObjectClass *klass, const char *name,
                                           ObjectPropertyRelease *release,
                                           void *opaque);
 
+/**
+ * object_property_set_default:
+ * @prop: the property to set
+ * @value: the value to be written to the property
+ *
+ * Set the property default value.
+ *
+ * Ownership of @value is transferred to the property.
+ */
+void object_property_set_default(ObjectProperty *prop, QObject *value);
+
 /**
  * object_property_set_default_bool:
  * @prop: the property to set
diff --git a/qom/object.c b/qom/object.c
index 7c11bcd3b1..6b0d9d8c79 100644
--- a/qom/object.c
+++ b/qom/object.c
@@ -1547,7 +1547,7 @@ static void object_property_init_defval(Object *obj, ObjectProperty *prop)
     visit_free(v);
 }
 
-static void object_property_set_default(ObjectProperty *prop, QObject *defval)
+void object_property_set_default(ObjectProperty *prop, QObject *defval)
 {
     assert(!prop->defval);
     assert(!prop->init);
-- 
2.28.0

[PATCH v3 16/19] qom: Make PropertyInfo.set_default_value optional

[Eduardo Habkost]

If .set_default_value is not set, call
object_property_set_default().  This will let us delete most of
the .set_default_value functions later.

Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
This is a new patch in v3 of this series.

In v2 of the series, equivalent functionality was implemented by
"qom: Use qlit to represent property defaults".
---
 include/qom/field-property.h |  3 +++
 qom/field-property.c         | 12 ++++++++++--
 2 files changed, 13 insertions(+), 2 deletions(-)

diff --git a/include/qom/field-property.h b/include/qom/field-property.h
index 951cec2fb0..00b83ee9ba 100644
--- a/include/qom/field-property.h
+++ b/include/qom/field-property.h
@@ -53,6 +53,9 @@ struct PropertyInfo {
      * @set_default_value: Callback for initializing the default value
      *
      * @defval is a weak reference.
+     *
+     * Optional.  If not set and Property.defval is not QTYPE_NONE,
+     * object_property_set_default() will be called.
      */
     void (*set_default_value)(ObjectProperty *op, const Property *prop,
                               const QObject *defval);
diff --git a/qom/field-property.c b/qom/field-property.c
index 593ffb53e9..d21ff98862 100644
--- a/qom/field-property.c
+++ b/qom/field-property.c
@@ -72,8 +72,16 @@ static void field_prop_set_default_value(ObjectProperty *op,
     }
 
     defval = qobject_from_qlit(&prop->defval);
-    assert(prop->info->set_default_value);
-    prop->info->set_default_value(op, prop, defval);
+    if (prop->info->set_default_value) {
+        /* .set_default_value() gets a weak reference */
+        prop->info->set_default_value(op, prop, defval);
+    } else {
+        /*
+         * object_property_set_default() takes ownership,
+         * so qobject_ref() is needed.
+         */
+        object_property_set_default(op, qobject_ref(defval));
+    }
     qobject_unref(defval);
 }
 
-- 
2.28.0

[PATCH v3 17/19] qom: Delete field_prop_set_default_value_*int()

[Eduardo Habkost]

The field_prop_set_default_value_*int() functions can be
completely replaced by object_propert_set_default(),
we don't need them anymore.

Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
This is a new patch in v3 of this series.

In v2 of the series, equivalent changes were part of "qom: Use
qlit to represent property defaults".
---
 include/qom/field-property-internal.h |  6 ------
 hw/core/qdev-properties-system.c      |  2 --
 qom/property-types.c                  | 25 -------------------------
 3 files changed, 33 deletions(-)

diff --git a/include/qom/field-property-internal.h b/include/qom/field-property-internal.h
index 7ed0d8d160..4bcf5b45f3 100644
--- a/include/qom/field-property-internal.h
+++ b/include/qom/field-property-internal.h
@@ -16,12 +16,6 @@ void field_prop_set_enum(Object *obj, Visitor *v, const char *name,
 void field_prop_set_default_value_enum(ObjectProperty *op,
                                        const Property *prop,
                                        const QObject *defval);
-void field_prop_set_default_value_int(ObjectProperty *op,
-                                      const Property *prop,
-                                       const QObject *defval);
-void field_prop_set_default_value_uint(ObjectProperty *op,
-                                       const Property *prop,
-                                       const QObject *defval);
 
 void field_prop_get_int32(Object *obj, Visitor *v, const char *name,
                           void *opaque, Error **errp);
diff --git a/hw/core/qdev-properties-system.c b/hw/core/qdev-properties-system.c
index 117c540254..b2df955f2a 100644
--- a/hw/core/qdev-properties-system.c
+++ b/hw/core/qdev-properties-system.c
@@ -571,7 +571,6 @@ const PropertyInfo qdev_prop_blocksize = {
                    " and " MAX_BLOCK_SIZE_STR,
     .get   = field_prop_get_size32,
     .set   = set_blocksize,
-    .set_default_value = field_prop_set_default_value_uint,
 };
 
 /* --- Block device error handling policy --- */
@@ -769,7 +768,6 @@ const PropertyInfo qdev_prop_pci_devfn = {
     .print = print_pci_devfn,
     .get   = field_prop_get_int32,
     .set   = set_pci_devfn,
-    .set_default_value = field_prop_set_default_value_int,
 };
 
 /* --- pci host address --- */
diff --git a/qom/property-types.c b/qom/property-types.c
index 0fc24b3687..399e36c29e 100644
--- a/qom/property-types.c
+++ b/qom/property-types.c
@@ -195,27 +195,10 @@ static void set_uint8(Object *obj, Visitor *v, const char *name, void *opaque,
     visit_type_uint8(v, name, ptr, errp);
 }
 
-void field_prop_set_default_value_int(ObjectProperty *op,
-                                      const Property *prop,
-                                      const QObject *defval)
-{
-    QNum *qn = qobject_to(QNum, defval);
-    object_property_set_default_int(op, qnum_get_int(qn));
-}
-
-void field_prop_set_default_value_uint(ObjectProperty *op,
-                                       const Property *prop,
-                                       const QObject *defval)
-{
-    QNum *qn = qobject_to(QNum, defval);
-    object_property_set_default_uint(op, qnum_get_uint(qn));
-}
-
 const PropertyInfo prop_info_uint8 = {
     .name  = "uint8",
     .get   = get_uint8,
     .set   = set_uint8,
-    .set_default_value = field_prop_set_default_value_uint,
 };
 
 /* --- 16bit integer --- */
@@ -242,7 +225,6 @@ const PropertyInfo prop_info_uint16 = {
     .name  = "uint16",
     .get   = get_uint16,
     .set   = set_uint16,
-    .set_default_value = field_prop_set_default_value_uint,
 };
 
 /* --- 32bit integer --- */
@@ -287,14 +269,12 @@ const PropertyInfo prop_info_uint32 = {
     .name  = "uint32",
     .get   = get_uint32,
     .set   = set_uint32,
-    .set_default_value = field_prop_set_default_value_uint,
 };
 
 const PropertyInfo prop_info_int32 = {
     .name  = "int32",
     .get   = field_prop_get_int32,
     .set   = set_int32,
-    .set_default_value = field_prop_set_default_value_int,
 };
 
 /* --- 64bit integer --- */
@@ -339,14 +319,12 @@ const PropertyInfo prop_info_uint64 = {
     .name  = "uint64",
     .get   = get_uint64,
     .set   = set_uint64,
-    .set_default_value = field_prop_set_default_value_uint,
 };
 
 const PropertyInfo prop_info_int64 = {
     .name  = "int64",
     .get   = get_int64,
     .set   = set_int64,
-    .set_default_value = field_prop_set_default_value_int,
 };
 
 /* --- string --- */
@@ -441,7 +419,6 @@ const PropertyInfo prop_info_size32 = {
     .name  = "size",
     .get = field_prop_get_size32,
     .set = set_size32,
-    .set_default_value = field_prop_set_default_value_uint,
 };
 
 /* --- support for array properties --- */
@@ -505,7 +482,6 @@ const PropertyInfo prop_info_arraylen = {
     .name = "uint32",
     .get = get_uint32,
     .set = set_prop_arraylen,
-    .set_default_value = field_prop_set_default_value_uint,
 };
 
 /* --- 64bit unsigned int 'size' type --- */
@@ -532,7 +508,6 @@ const PropertyInfo prop_info_size = {
     .name  = "size",
     .get = get_size,
     .set = set_size,
-    .set_default_value = field_prop_set_default_value_uint,
 };
 
 /* --- object link property --- */
-- 
2.28.0

[PATCH v3 18/19] qom: Delete set_default_uuid()

[Eduardo Habkost]

The function can be completely replaced by
object_property_set_default(), we don't need it anymore.

Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
This is a new patch in v3 of this series.

In v2 of the series, equivalent changes were part of "qom: Use
qlit to represent property defaults".
---
 hw/core/qdev-properties-system.c | 8 --------
 1 file changed, 8 deletions(-)

diff --git a/hw/core/qdev-properties-system.c b/hw/core/qdev-properties-system.c
index b2df955f2a..07945ea1c0 100644
--- a/hw/core/qdev-properties-system.c
+++ b/hw/core/qdev-properties-system.c
@@ -1077,18 +1077,10 @@ static void set_uuid(Object *obj, Visitor *v, const char *name, void *opaque,
     g_free(str);
 }
 
-static void set_default_uuid(ObjectProperty *op, const Property *prop,
-                                  const QObject *defval)
-{
-    QString *qs = qobject_to(QString, defval);
-    object_property_set_default_str(op, qstring_get_str(qs));
-}
-
 const PropertyInfo qdev_prop_uuid = {
     .name  = "str",
     .description = "UUID (aka GUID) or \"" UUID_VALUE_AUTO
         "\" for random value",
     .get   = get_uuid,
     .set   = set_uuid,
-    .set_default_value = set_default_uuid,
 };
-- 
2.28.0

[PATCH v3 19/19] qom: Delete set_default_value_bool()

[Eduardo Habkost]

set_default_value_bool() can be completely replaced by
object_property_set_default(), we don't need that function
anymore.

Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
This is a new patch in v3 of this series.

In v2 of the series, equivalent changes were part of "qom: Use
qlit to represent property defaults".
---
 qom/property-types.c | 10 ----------
 1 file changed, 10 deletions(-)

diff --git a/qom/property-types.c b/qom/property-types.c
index 399e36c29e..cb7ba2a229 100644
--- a/qom/property-types.c
+++ b/qom/property-types.c
@@ -84,19 +84,11 @@ static void prop_set_bit(Object *obj, Visitor *v, const char *name,
     bit_prop_set(obj, prop, value);
 }
 
-static void set_default_value_bool(ObjectProperty *op, const Property *prop,
-                                   const QObject *defval)
-{
-    QBool *qb = qobject_to(QBool, defval);
-    object_property_set_default_bool(op, qbool_get_bool(qb));
-}
-
 const PropertyInfo prop_info_bit = {
     .name  = "bool",
     .description = "on/off",
     .get   = prop_get_bit,
     .set   = prop_set_bit,
-    .set_default_value = set_default_value_bool,
 };
 
 /* Bit64 */
@@ -145,7 +137,6 @@ const PropertyInfo prop_info_bit64 = {
     .description = "on/off",
     .get   = prop_get_bit64,
     .set   = prop_set_bit64,
-    .set_default_value = set_default_value_bool,
 };
 
 /* --- bool --- */
@@ -172,7 +163,6 @@ const PropertyInfo prop_info_bool = {
     .name  = "bool",
     .get   = get_bool,
     .set   = set_bool,
-    .set_default_value = set_default_value_bool,
 };
 
 /* --- 8bit integer --- */
-- 
2.28.0