[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v2 09/45] qapi: merge QInt and QFloat in QNum
|
From: |
Markus Armbruster |
|
Subject: |
Re: [Qemu-devel] [PATCH v2 09/45] qapi: merge QInt and QFloat in QNum |
|
Date: |
Fri, 02 Jun 2017 09:03:22 +0200 |
|
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/25.2 (gnu/linux) |
Marc-André Lureau <address@hidden> writes:
> We would like to use a same QObject type to represent numbers, whether
> they are int, uint, or floats. Getters will allow some compatibility
> between the various types if the number fits other representations.
>
> Signed-off-by: Marc-André Lureau <address@hidden>
> ---
> scripts/qapi.py | 40 ++++-----
> scripts/qapi-visit.py | 2 +-
> include/qapi/qmp/qdict.h | 3 +-
> include/qapi/qmp/qfloat.h | 29 -------
> include/qapi/qmp/qint.h | 28 ------
> include/qapi/qmp/qlist.h | 3 +-
> include/qapi/qmp/qnum.h | 44 ++++++++++
> include/qapi/qmp/types.h | 3 +-
> include/qapi/qobject-input-visitor.h | 6 +-
> include/qapi/qobject-output-visitor.h | 8 +-
> block/blkdebug.c | 1 -
> block/nbd.c | 1 -
> block/nfs.c | 1 -
> block/qapi.c | 13 ++-
> block/quorum.c | 1 -
> block/sheepdog.c | 1 -
> block/ssh.c | 1 -
> block/vvfat.c | 1 -
> blockdev.c | 8 +-
> hw/acpi/pcihp.c | 1 -
> hw/i386/acpi-build.c | 41 +++++++--
> hw/usb/xen-usb.c | 1 -
> monitor.c | 2 +-
> qapi/qobject-input-visitor.c | 41 +++------
> qapi/qobject-output-visitor.c | 6 +-
> qga/commands.c | 2 +-
> qga/main.c | 1 -
> qobject/json-parser.c | 29 +++----
> qobject/qdict.c | 43 +++++-----
> qobject/qfloat.c | 62 --------------
> qobject/qint.c | 61 -------------
> qobject/qjson.c | 37 +-------
> qobject/qnum.c | 141
> +++++++++++++++++++++++++++++++
> qobject/qobject.c | 3 +-
> qom/object.c | 16 ++--
> target/i386/cpu.c | 6 +-
> tests/check-qdict.c | 30 ++++---
> tests/check-qfloat.c | 53 ------------
> tests/check-qint.c | 87 -------------------
> tests/check-qjson.c | 91 +++++++++++---------
> tests/check-qlist.c | 18 ++--
> tests/check-qnum.c | 133 +++++++++++++++++++++++++++++
> tests/test-qmp-commands.c | 8 +-
> tests/test-qmp-event.c | 9 +-
> tests/test-qobject-input-visitor.c | 33 ++++----
> tests/test-qobject-output-visitor.c | 66 +++++++++------
> tests/test-x86-cpuid-compat.c | 20 +++--
> ui/spice-core.c | 1 -
> ui/vnc-enc-tight.c | 1 -
> util/qemu-option.c | 20 ++---
> MAINTAINERS | 3 +-
> qobject/Makefile.objs | 2 +-
> scripts/coccinelle/qobject.cocci | 4 +-
> tests/.gitignore | 3 +-
> tests/Makefile.include | 13 ++-
> tests/qapi-schema/comments.out | 2 +-
> tests/qapi-schema/doc-good.out | 2 +-
> tests/qapi-schema/empty.out | 2 +-
> tests/qapi-schema/event-case.out | 2 +-
> tests/qapi-schema/ident-with-escape.out | 2 +-
> tests/qapi-schema/include-relpath.out | 2 +-
> tests/qapi-schema/include-repetition.out | 2 +-
> tests/qapi-schema/include-simple.out | 2 +-
> tests/qapi-schema/indented-expr.out | 2 +-
> tests/qapi-schema/qapi-schema-test.out | 2 +-
> 65 files changed, 637 insertions(+), 665 deletions(-)
> delete mode 100644 include/qapi/qmp/qfloat.h
> delete mode 100644 include/qapi/qmp/qint.h
> create mode 100644 include/qapi/qmp/qnum.h
> delete mode 100644 qobject/qfloat.c
> delete mode 100644 qobject/qint.c
> create mode 100644 qobject/qnum.c
> delete mode 100644 tests/check-qfloat.c
> delete mode 100644 tests/check-qint.c
> create mode 100644 tests/check-qnum.c
>
> diff --git a/scripts/qapi.py b/scripts/qapi.py
> index 06e583d8c3..0de809f56b 100644
> --- a/scripts/qapi.py
> +++ b/scripts/qapi.py
> @@ -21,18 +21,18 @@ from ordereddict import OrderedDict
>
> builtin_types = {
> 'str': 'QTYPE_QSTRING',
> - 'int': 'QTYPE_QINT',
> - 'number': 'QTYPE_QFLOAT',
> + 'int': 'QTYPE_QNUM',
> + 'number': 'QTYPE_QNUM',
> 'bool': 'QTYPE_QBOOL',
> - 'int8': 'QTYPE_QINT',
> - 'int16': 'QTYPE_QINT',
> - 'int32': 'QTYPE_QINT',
> - 'int64': 'QTYPE_QINT',
> - 'uint8': 'QTYPE_QINT',
> - 'uint16': 'QTYPE_QINT',
> - 'uint32': 'QTYPE_QINT',
> - 'uint64': 'QTYPE_QINT',
> - 'size': 'QTYPE_QINT',
> + 'int8': 'QTYPE_QNUM',
> + 'int16': 'QTYPE_QNUM',
> + 'int32': 'QTYPE_QNUM',
> + 'int64': 'QTYPE_QNUM',
> + 'uint8': 'QTYPE_QNUM',
> + 'uint16': 'QTYPE_QNUM',
> + 'uint32': 'QTYPE_QNUM',
> + 'uint64': 'QTYPE_QNUM',
> + 'size': 'QTYPE_QNUM',
> 'any': None, # any QType possible, actually
> 'QType': 'QTYPE_QSTRING',
> }
> @@ -820,16 +820,10 @@ def check_alternate(expr, info):
> if v in ['on', 'off']:
> conflicting.add('QTYPE_QBOOL')
> if re.match(r'[-+0-9.]', v): # lazy, could be tightened
> - conflicting.add('QTYPE_QINT')
> - conflicting.add('QTYPE_QFLOAT')
> + conflicting.add('QTYPE_QNUM')
> else:
> - conflicting.add('QTYPE_QINT')
> - conflicting.add('QTYPE_QFLOAT')
> + conflicting.add('QTYPE_QNUM')
> conflicting.add('QTYPE_QBOOL')
> - elif qtype == 'QTYPE_QINT':
> - conflicting.add('QTYPE_QFLOAT')
> - elif qtype == 'QTYPE_QFLOAT':
> - conflicting.add('QTYPE_QINT')
> if conflicting & set(types_seen):
> raise QAPISemError(info, "Alternate '%s' member '%s' can't "
> "be distinguished from member '%s'"
> @@ -1063,8 +1057,8 @@ class QAPISchemaType(QAPISchemaEntity):
> def alternate_qtype(self):
> json2qtype = {
> 'string': 'QTYPE_QSTRING',
> - 'number': 'QTYPE_QFLOAT',
> - 'int': 'QTYPE_QINT',
> + 'number': 'QTYPE_QNUM',
> + 'int': 'QTYPE_QNUM',
> 'boolean': 'QTYPE_QBOOL',
> 'object': 'QTYPE_QDICT'
> }
> @@ -1526,9 +1520,9 @@ class QAPISchema(object):
> self.the_empty_object_type = QAPISchemaObjectType(
> 'q_empty', None, None, None, [], None)
> self._def_entity(self.the_empty_object_type)
> - qtype_values = self._make_enum_members(['none', 'qnull', 'qint',
> + qtype_values = self._make_enum_members(['none', 'qnull', 'qnum',
> 'qstring', 'qdict', 'qlist',
> - 'qfloat', 'qbool'])
> + 'qbool'])
> self._def_entity(QAPISchemaEnumType('QType', None, None,
> qtype_values, 'QTYPE'))
>
> diff --git a/scripts/qapi-visit.py b/scripts/qapi-visit.py
> index 5737aefa05..cc447ecacc 100644
> --- a/scripts/qapi-visit.py
> +++ b/scripts/qapi-visit.py
> @@ -164,7 +164,7 @@ def gen_visit_alternate(name, variants):
> promote_int = 'true'
> ret = ''
> for var in variants.variants:
> - if var.type.alternate_qtype() == 'QTYPE_QINT':
> + if var.type.alternate_qtype() == 'QTYPE_QNUM':
> promote_int = 'false'
>
> ret += mcgen('''
> diff --git a/include/qapi/qmp/qdict.h b/include/qapi/qmp/qdict.h
> index 188440a6a8..363e431106 100644
> --- a/include/qapi/qmp/qdict.h
> +++ b/include/qapi/qmp/qdict.h
> @@ -15,6 +15,7 @@
>
> #include "qapi/qmp/qobject.h"
> #include "qapi/qmp/qlist.h"
> +#include "qapi/qmp/qnum.h"
> #include "qemu/queue.h"
>
> #define QDICT_BUCKET_MAX 512
> @@ -54,7 +55,7 @@ void qdict_destroy_obj(QObject *obj);
>
> /* Helpers for int, bool, and string */
> #define qdict_put_int(qdict, key, value) \
> - qdict_put(qdict, key, qint_from_int(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) \
> diff --git a/include/qapi/qmp/qfloat.h b/include/qapi/qmp/qfloat.h
> deleted file mode 100644
> index b5d15836b5..0000000000
> --- a/include/qapi/qmp/qfloat.h
> +++ /dev/null
> @@ -1,29 +0,0 @@
> -/*
> - * QFloat Module
> - *
> - * Copyright IBM, Corp. 2009
> - *
> - * Authors:
> - * Anthony Liguori <address@hidden>
> - *
> - * This work is licensed under the terms of the GNU LGPL, version 2.1 or
> later.
> - * See the COPYING.LIB file in the top-level directory.
> - *
> - */
> -
> -#ifndef QFLOAT_H
> -#define QFLOAT_H
> -
> -#include "qapi/qmp/qobject.h"
> -
> -typedef struct QFloat {
> - QObject base;
> - double value;
> -} QFloat;
> -
> -QFloat *qfloat_from_double(double value);
> -double qfloat_get_double(const QFloat *qi);
> -QFloat *qobject_to_qfloat(const QObject *obj);
> -void qfloat_destroy_obj(QObject *obj);
> -
> -#endif /* QFLOAT_H */
> diff --git a/include/qapi/qmp/qint.h b/include/qapi/qmp/qint.h
> deleted file mode 100644
> index 3aaff768dd..0000000000
> --- a/include/qapi/qmp/qint.h
> +++ /dev/null
> @@ -1,28 +0,0 @@
> -/*
> - * QInt Module
> - *
> - * Copyright (C) 2009 Red Hat Inc.
> - *
> - * Authors:
> - * Luiz Capitulino <address@hidden>
> - *
> - * This work is licensed under the terms of the GNU LGPL, version 2.1 or
> later.
> - * See the COPYING.LIB file in the top-level directory.
> - */
> -
> -#ifndef QINT_H
> -#define QINT_H
> -
> -#include "qapi/qmp/qobject.h"
> -
> -typedef struct QInt {
> - QObject base;
> - int64_t value;
> -} QInt;
> -
> -QInt *qint_from_int(int64_t value);
> -int64_t qint_get_int(const QInt *qi);
> -QInt *qobject_to_qint(const QObject *obj);
> -void qint_destroy_obj(QObject *obj);
> -
> -#endif /* QINT_H */
> diff --git a/include/qapi/qmp/qlist.h b/include/qapi/qmp/qlist.h
> index 5dc4ed9616..c4b5fdad9b 100644
> --- a/include/qapi/qmp/qlist.h
> +++ b/include/qapi/qmp/qlist.h
> @@ -14,6 +14,7 @@
> #define QLIST_H
>
> #include "qapi/qmp/qobject.h"
> +#include "qapi/qmp/qnum.h"
> #include "qemu/queue.h"
>
> typedef struct QListEntry {
> @@ -31,7 +32,7 @@ typedef struct QList {
>
> /* Helpers for int, bool, and string */
> #define qlist_append_int(qlist, value) \
> - qlist_append(qlist, qint_from_int(value))
> + qlist_append(qlist, qnum_from_int(value))
> #define qlist_append_bool(qlist, value) \
> qlist_append(qlist, qbool_from_bool(value))
> #define qlist_append_str(qlist, value) \
> diff --git a/include/qapi/qmp/qnum.h b/include/qapi/qmp/qnum.h
> new file mode 100644
> index 0000000000..cc636fac5f
> --- /dev/null
> +++ b/include/qapi/qmp/qnum.h
> @@ -0,0 +1,44 @@
> +/*
> + * QNum Module
> + *
> + * Copyright (C) 2009 Red Hat Inc.
> + *
> + * Authors:
> + * Luiz Capitulino <address@hidden>
> + * Marc-André Lureau <address@hidden>
> + *
> + * This work is licensed under the terms of the GNU LGPL, version 2.1 or
> later.
> + * See the COPYING.LIB file in the top-level directory.
> + */
> +
> +#ifndef QNUM_H
> +#define QNUM_H
> +
> +#include "qapi/qmp/qobject.h"
> +
> +typedef enum {
> + QNUM_I64,
> + QNUM_DOUBLE
> +} QNumKind;
> +
> +typedef struct QNum {
> + QObject base;
> + QNumKind kind;
> + union {
> + int64_t i64;
> + double dbl;
> + } u;
> +} QNum;
> +
> +QNum *qnum_from_int(int64_t value);
> +QNum *qnum_from_double(double value);
> +
> +bool qnum_get_int(const QNum *qn, int64_t *val);
> +double qnum_get_double(QNum *qn);
Your new qnum_get_int() interface makes sense, but the asymmetry bothers
me. Hmm.
> +
> +char *qnum_to_string(QNum *qn);
> +
> +QNum *qobject_to_qnum(const QObject *obj);
> +void qnum_destroy_obj(QObject *obj);
> +
> +#endif /* QNUM_H */
> diff --git a/include/qapi/qmp/types.h b/include/qapi/qmp/types.h
> index 27cfbd84e5..a4bc662bfb 100644
> --- a/include/qapi/qmp/types.h
> +++ b/include/qapi/qmp/types.h
> @@ -14,8 +14,7 @@
> #define QAPI_QMP_TYPES_H
>
> #include "qapi/qmp/qobject.h"
> -#include "qapi/qmp/qint.h"
> -#include "qapi/qmp/qfloat.h"
> +#include "qapi/qmp/qnum.h"
> #include "qapi/qmp/qbool.h"
> #include "qapi/qmp/qstring.h"
> #include "qapi/qmp/qdict.h"
> diff --git a/include/qapi/qobject-input-visitor.h
> b/include/qapi/qobject-input-visitor.h
> index b399285c43..daee18c6ac 100644
> --- a/include/qapi/qobject-input-visitor.h
> +++ b/include/qapi/qobject-input-visitor.h
> @@ -30,9 +30,9 @@ typedef struct QObjectInputVisitor QObjectInputVisitor;
> * visit_type_FOO() creates an instance of QAPI type FOO. The visited
> * QObject must match FOO. QDict matches struct/union types, QList
> * matches list types, QString matches type 'str' and enumeration
> - * types, QInt matches integer types, QFloat matches type 'number',
> - * QBool matches type 'bool'. Type 'any' is matched by QObject. A
> - * QAPI alternate type is matched when one of its member types is.
> + * types, QNum matches integer and float types, QBool matches type
> + * 'bool'. Type 'any' is matched by QObject. A QAPI alternate type
> + * is matched when one of its member types is.
> *
> * visit_start_struct() ... visit_end_struct() visits a QDict and
> * creates a QAPI struct/union. Visits in between visit the
> diff --git a/include/qapi/qobject-output-visitor.h
> b/include/qapi/qobject-output-visitor.h
> index 9b990c318e..e5a3490812 100644
> --- a/include/qapi/qobject-output-visitor.h
> +++ b/include/qapi/qobject-output-visitor.h
> @@ -28,10 +28,10 @@ typedef struct QObjectOutputVisitor QObjectOutputVisitor;
> *
> * visit_type_FOO() creates a QObject for QAPI type FOO. It creates a
> * QDict for struct/union types, a QList for list types, QString for
> - * type 'str' and enumeration types, QInt for integer types, QFloat
> - * for type 'number', QBool for type 'bool'. For type 'any', it
> - * increments the QObject's reference count. For QAPI alternate
> - * types, it creates the QObject for the member that is in use.
> + * type 'str' and enumeration types, QNum for integer and float
> + * types, QBool for type 'bool'. For type 'any', it increments the
> + * QObject's reference count. For QAPI alternate types, it creates
> + * the QObject for the member that is in use.
> *
> * visit_start_struct() ... visit_end_struct() visits a QAPI
> * struct/union and creates a QDict. Visits in between visit the
> diff --git a/block/blkdebug.c b/block/blkdebug.c
> index a5196e889d..0618fc71c6 100644
> --- a/block/blkdebug.c
> +++ b/block/blkdebug.c
> @@ -31,7 +31,6 @@
> #include "qemu/module.h"
> #include "qapi/qmp/qbool.h"
> #include "qapi/qmp/qdict.h"
> -#include "qapi/qmp/qint.h"
> #include "qapi/qmp/qstring.h"
> #include "sysemu/qtest.h"
>
> diff --git a/block/nbd.c b/block/nbd.c
> index 975faab2c5..e946ea944d 100644
> --- a/block/nbd.c
> +++ b/block/nbd.c
> @@ -37,7 +37,6 @@
> #include "qapi/qobject-output-visitor.h"
> #include "qapi/qmp/qdict.h"
> #include "qapi/qmp/qjson.h"
> -#include "qapi/qmp/qint.h"
> #include "qapi/qmp/qstring.h"
> #include "qemu/cutils.h"
>
> diff --git a/block/nfs.c b/block/nfs.c
> index 848b2c0bb0..decefd15f1 100644
> --- a/block/nfs.c
> +++ b/block/nfs.c
> @@ -36,7 +36,6 @@
> #include "qemu/cutils.h"
> #include "sysemu/sysemu.h"
> #include "qapi/qmp/qdict.h"
> -#include "qapi/qmp/qint.h"
> #include "qapi/qmp/qstring.h"
> #include "qapi-visit.h"
> #include "qapi/qobject-input-visitor.h"
> diff --git a/block/qapi.c b/block/qapi.c
> index a40922ea26..2050df29e4 100644
> --- a/block/qapi.c
> +++ b/block/qapi.c
> @@ -595,9 +595,11 @@ static void dump_qobject(fprintf_function func_fprintf,
> void *f,
> int comp_indent, QObject *obj)
> {
> switch (qobject_type(obj)) {
> - case QTYPE_QINT: {
> - QInt *value = qobject_to_qint(obj);
> - func_fprintf(f, "%" PRId64, qint_get_int(value));
> + case QTYPE_QNUM: {
> + QNum *value = qobject_to_qnum(obj);
> + char *tmp = qnum_to_string(value);
> + func_fprintf(f, "%s", tmp);
> + g_free(tmp);
> break;
> }
> case QTYPE_QSTRING: {
> @@ -615,11 +617,6 @@ static void dump_qobject(fprintf_function func_fprintf,
> void *f,
> dump_qlist(func_fprintf, f, comp_indent, value);
> break;
> }
> - case QTYPE_QFLOAT: {
> - QFloat *value = qobject_to_qfloat(obj);
> - func_fprintf(f, "%g", qfloat_get_double(value));
> - break;
> - }
> case QTYPE_QBOOL: {
> QBool *value = qobject_to_qbool(obj);
> func_fprintf(f, "%s", qbool_get_bool(value) ? "true" : "false");
> diff --git a/block/quorum.c b/block/quorum.c
> index 1b2a8c3937..55ba916655 100644
> --- a/block/quorum.c
> +++ b/block/quorum.c
> @@ -19,7 +19,6 @@
> #include "qapi/qmp/qbool.h"
> #include "qapi/qmp/qdict.h"
> #include "qapi/qmp/qerror.h"
> -#include "qapi/qmp/qint.h"
> #include "qapi/qmp/qjson.h"
> #include "qapi/qmp/qlist.h"
> #include "qapi/qmp/qstring.h"
> diff --git a/block/sheepdog.c b/block/sheepdog.c
> index a18315a1ca..dea9000bdd 100644
> --- a/block/sheepdog.c
> +++ b/block/sheepdog.c
> @@ -16,7 +16,6 @@
> #include "qapi-visit.h"
> #include "qapi/error.h"
> #include "qapi/qmp/qdict.h"
> -#include "qapi/qmp/qint.h"
> #include "qapi/qobject-input-visitor.h"
> #include "qemu/uri.h"
> #include "qemu/error-report.h"
> diff --git a/block/ssh.c b/block/ssh.c
> index 11203fc5a2..bac3453c3e 100644
> --- a/block/ssh.c
> +++ b/block/ssh.c
> @@ -34,7 +34,6 @@
> #include "qemu/sockets.h"
> #include "qemu/uri.h"
> #include "qapi-visit.h"
> -#include "qapi/qmp/qint.h"
> #include "qapi/qmp/qstring.h"
> #include "qapi/qobject-input-visitor.h"
> #include "qapi/qobject-output-visitor.h"
> diff --git a/block/vvfat.c b/block/vvfat.c
> index 426ca70e35..8ab647c0c6 100644
> --- a/block/vvfat.c
> +++ b/block/vvfat.c
> @@ -29,7 +29,6 @@
> #include "qemu/module.h"
> #include "qemu/bswap.h"
> #include "migration/blocker.h"
> -#include "qapi/qmp/qint.h"
> #include "qapi/qmp/qbool.h"
> #include "qapi/qmp/qstring.h"
> #include "qemu/cutils.h"
> diff --git a/blockdev.c b/blockdev.c
> index 892d768574..be18271c11 100644
> --- a/blockdev.c
> +++ b/blockdev.c
> @@ -334,9 +334,11 @@ static bool parse_stats_intervals(BlockAcctStats *stats,
> QList *intervals,
> break;
> }
>
> - case QTYPE_QINT: {
> - int64_t length = qint_get_int(qobject_to_qint(entry->value));
> - if (length > 0 && length <= UINT_MAX) {
> + case QTYPE_QNUM: {
> + int64_t length;
> +
> + if (qnum_get_int(qobject_to_qnum(entry->value), &length) &&
> + length > 0 && length <= UINT_MAX) {
> block_acct_add_interval(stats, (unsigned) length);
> } else {
> error_setg(errp, "Invalid interval length: %" PRId64,
> length);
No longer crashes when entr->value is a floating-point number. Still
crashes when it isn't a number. Since neither can happen, there's no
real need to note this in the commit message. Okay.
> diff --git a/hw/acpi/pcihp.c b/hw/acpi/pcihp.c
> index 2b0f3e1bfb..3a531a4416 100644
> --- a/hw/acpi/pcihp.c
> +++ b/hw/acpi/pcihp.c
> @@ -37,7 +37,6 @@
> #include "hw/pci/pci_bus.h"
> #include "qapi/error.h"
> #include "qom/qom-qobject.h"
> -#include "qapi/qmp/qint.h"
>
> //#define DEBUG
>
> diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
> index 82bd44f38e..1709efdf1c 100644
> --- a/hw/i386/acpi-build.c
> +++ b/hw/i386/acpi-build.c
> @@ -57,7 +57,6 @@
>
> #include "hw/acpi/aml-build.h"
>
> -#include "qapi/qmp/qint.h"
> #include "qom/qom-qobject.h"
> #include "hw/i386/amd_iommu.h"
> #include "hw/i386/intel_iommu.h"
> @@ -129,6 +128,7 @@ static void acpi_get_pm_info(AcpiPmInfo *pm)
> Object *lpc = ich9_lpc_find();
> Object *obj = NULL;
> QObject *o;
> + int64_t val;
>
> pm->cpu_hp_io_base = 0;
> pm->pcihp_io_base = 0;
> @@ -150,21 +150,30 @@ static void acpi_get_pm_info(AcpiPmInfo *pm)
> /* Fill in optional s3/s4 related properties */
> o = object_property_get_qobject(obj, ACPI_PM_PROP_S3_DISABLED, NULL);
> if (o) {
> - pm->s3_disabled = qint_get_int(qobject_to_qint(o));
> + if (!qnum_get_int(qobject_to_qnum(o), &val)) {
> + g_assert_not_reached();
> + }
> + pm->s3_disabled = val;
> } else {
> pm->s3_disabled = false;
> }
> qobject_decref(o);
> o = object_property_get_qobject(obj, ACPI_PM_PROP_S4_DISABLED, NULL);
> if (o) {
> - pm->s4_disabled = qint_get_int(qobject_to_qint(o));
> + if (!qnum_get_int(qobject_to_qnum(o), &val)) {
> + g_assert_not_reached();
> + }
> + pm->s4_disabled = val;
> } else {
> pm->s4_disabled = false;
> }
> qobject_decref(o);
> o = object_property_get_qobject(obj, ACPI_PM_PROP_S4_VAL, NULL);
> if (o) {
> - pm->s4_val = qint_get_int(qobject_to_qint(o));
> + if (!qnum_get_int(qobject_to_qnum(o), &val)) {
> + g_assert_not_reached();
> + }
> + pm->s4_val = val;
> } else {
> pm->s4_val = false;
> }
> @@ -529,7 +538,11 @@ static void build_append_pci_bus_devices(Aml
> *parent_scope, PCIBus *bus,
>
> bsel = object_property_get_qobject(OBJECT(bus), ACPI_PCIHP_PROP_BSEL,
> NULL);
> if (bsel) {
> - int64_t bsel_val = qint_get_int(qobject_to_qint(bsel));
> + int64_t bsel_val;
> +
> + if (!qnum_get_int(qobject_to_qnum(bsel), &bsel_val)) {
> + g_assert_not_reached();
> + }
>
> aml_append(parent_scope, aml_name_decl("BSEL", aml_int(bsel_val)));
> notify_method = aml_method("DVNT", 2, AML_NOTSERIALIZED);
> @@ -639,7 +652,12 @@ static void build_append_pci_bus_devices(Aml
> *parent_scope, PCIBus *bus,
>
> /* If bus supports hotplug select it and notify about local events */
> if (bsel) {
> - int64_t bsel_val = qint_get_int(qobject_to_qint(bsel));
> + int64_t bsel_val;
> +
> + if (!qnum_get_int(qobject_to_qnum(bsel), &bsel_val)) {
> + g_assert_not_reached();
> + }
> +
> aml_append(method, aml_store(aml_int(bsel_val), aml_name("BNUM")));
> aml_append(method,
> aml_call2("DVNT", aml_name("PCIU"), aml_int(1) /* Device Check
> */)
> @@ -2607,6 +2625,7 @@ static bool acpi_get_mcfg(AcpiMcfgInfo *mcfg)
> {
> Object *pci_host;
> QObject *o;
> + int64_t val;
>
> pci_host = acpi_get_i386_pci_host();
> g_assert(pci_host);
> @@ -2615,12 +2634,18 @@ static bool acpi_get_mcfg(AcpiMcfgInfo *mcfg)
> if (!o) {
> return false;
> }
> - mcfg->mcfg_base = qint_get_int(qobject_to_qint(o));
> + if (!qnum_get_int(qobject_to_qnum(o), &val)) {
> + g_assert_not_reached();
> + }
> + mcfg->mcfg_base = val;
> qobject_decref(o);
>
> o = object_property_get_qobject(pci_host, PCIE_HOST_MCFG_SIZE, NULL);
> assert(o);
> - mcfg->mcfg_size = qint_get_int(qobject_to_qint(o));
> + if (!qnum_get_int(qobject_to_qnum(o), &val)) {
> + g_assert_not_reached();
> + }
> + mcfg->mcfg_size = val;
> qobject_decref(o);
> return true;
> }
We've seen this pattern
if (!qnum_get_int(qobject_to_qnum(src), &tmp)) {
g_assert_not_reached();
}
dst = tmp;
quite a few times now. Enough to justify a helper, I think.
Hmm, we could reduce the asymmetry that bothers me this way:
bool qnum_get_try_int(const QNum *qn, int64_t *val);
int64_t qnum_get_int(const QNum *qn);
double qnum_get_double(QNum *qn);
Bonus: less churn:
o = object_property_get_qobject(pci_host, PCIE_HOST_MCFG_SIZE, NULL);
assert(o);
- mcfg->mcfg_size = qint_get_int(qobject_to_qint(o));
+ mcfg->mcfg_size = qnum_get_int(qobject_to_qint(o));
qobject_decref(o);
return true;
What do you think?
Speaking of patterns. A substantial part of the patch is a mechanical
transformation
- qobject_to_qint(E)
+ qobject_to_qnum(E)
- qint_get_int(E)
+ qnum_get_int(E)
- LHS = qint_get_int(E)
+ if (!qnum_get_int(E, &TMP)
+ g_assert_not_reached();
+ }
+ LHS = TMP;
// plus several more variations of this pattern
- qobject_to_qfloat(E)
+ qobject_to_qnum(E)
- qfloat_get_double(E)
+ qnum_get_double(E)
plus replacement / merge of QInt / QFloat variables. Did you do it
entirely by hand or with help from Coccinelle?
Other patterns that might justify helpers:
qnum_get_double(qobject_to_qnum(OBJ))
qnum_get_int(qobject_to_qnum(OBJ), ERR)
qnum_get_int(qobject_to_qnum(OBJ), &error_abort)
Idea, not demand.
> diff --git a/hw/usb/xen-usb.c b/hw/usb/xen-usb.c
> index fe62183fe3..584a6f2442 100644
> --- a/hw/usb/xen-usb.c
> +++ b/hw/usb/xen-usb.c
> @@ -30,7 +30,6 @@
> #include "hw/xen/xen_backend.h"
> #include "monitor/qdev.h"
> #include "qapi/qmp/qbool.h"
> -#include "qapi/qmp/qint.h"
> #include "qapi/qmp/qstring.h"
>
> #include "hw/xen/io/ring.h"
> diff --git a/monitor.c b/monitor.c
> index baa73c98b7..a8ac971015 100644
> --- a/monitor.c
> +++ b/monitor.c
> @@ -2971,7 +2971,7 @@ static QDict *monitor_parse_arguments(Monitor *mon,
> monitor_printf(mon, "Unknown unit suffix\n");
> goto fail;
> }
> - qdict_put(qdict, key, qfloat_from_double(val));
> + qdict_put(qdict, key, qnum_from_double(val));
> }
> break;
> case 'b':
> diff --git a/qapi/qobject-input-visitor.c b/qapi/qobject-input-visitor.c
> index eac40f618a..b32318c352 100644
> --- a/qapi/qobject-input-visitor.c
> +++ b/qapi/qobject-input-visitor.c
> @@ -378,9 +378,6 @@ static void qobject_input_start_alternate(Visitor *v,
> const char *name,
> }
> *obj = g_malloc0(size);
> (*obj)->type = qobject_type(qobj);
> - if (promote_int && (*obj)->type == QTYPE_QINT) {
> - (*obj)->type = QTYPE_QFLOAT;
> - }
> }
>
> static void qobject_input_type_int64(Visitor *v, const char *name, int64_t
> *obj,
> @@ -388,22 +385,18 @@ static void qobject_input_type_int64(Visitor *v, const
> char *name, int64_t *obj,
> {
> QObjectInputVisitor *qiv = to_qiv(v);
> QObject *qobj = qobject_input_get_object(qiv, name, true, errp);
> - QInt *qint;
> + QNum *qnum;
>
> if (!qobj) {
> return;
> }
> - qint = qobject_to_qint(qobj);
> - if (!qint) {
> + qnum = qobject_to_qnum(qobj);
> + if (!qnum || !qnum_get_int(qnum, obj)) {
> error_setg(errp, QERR_INVALID_PARAMETER_TYPE,
> full_name(qiv, name), "integer");
> - return;
> }
> -
> - *obj = qint_get_int(qint);
> }
>
> -
> static void qobject_input_type_int64_keyval(Visitor *v, const char *name,
> int64_t *obj, Error **errp)
> {
> @@ -424,22 +417,21 @@ static void qobject_input_type_int64_keyval(Visitor *v,
> const char *name,
> static void qobject_input_type_uint64(Visitor *v, const char *name,
> uint64_t *obj, Error **errp)
> {
> - /* FIXME: qobject_to_qint mishandles values over INT64_MAX */
> + /* FIXME: qobject_to_qnum mishandles values over INT64_MAX */
> QObjectInputVisitor *qiv = to_qiv(v);
> QObject *qobj = qobject_input_get_object(qiv, name, true, errp);
> - QInt *qint;
> + QNum *qnum;
> + int64_t val;
>
> if (!qobj) {
> return;
> }
> - qint = qobject_to_qint(qobj);
> - if (!qint) {
> + qnum = qobject_to_qnum(qobj);
> + if (!qnum || !qnum_get_int(qnum, &val)) {
> error_setg(errp, QERR_INVALID_PARAMETER_TYPE,
> full_name(qiv, name), "integer");
> - return;
> }
> -
> - *obj = qint_get_int(qint);
> + *obj = val;
> }
>
> static void qobject_input_type_uint64_keyval(Visitor *v, const char *name,
> @@ -534,21 +526,14 @@ static void qobject_input_type_number(Visitor *v, const
> char *name, double *obj,
> {
> QObjectInputVisitor *qiv = to_qiv(v);
> QObject *qobj = qobject_input_get_object(qiv, name, true, errp);
> - QInt *qint;
> - QFloat *qfloat;
> + QNum *qnum;
>
> if (!qobj) {
> return;
> }
> - qint = qobject_to_qint(qobj);
> - if (qint) {
> - *obj = qint_get_int(qobject_to_qint(qobj));
> - return;
> - }
> -
> - qfloat = qobject_to_qfloat(qobj);
> - if (qfloat) {
> - *obj = qfloat_get_double(qobject_to_qfloat(qobj));
> + qnum = qobject_to_qnum(qobj);
> + if (qnum) {
> + *obj = qnum_get_double(qnum);
> return;
> }
>
error_setg(errp, QERR_INVALID_PARAMETER_TYPE,
full_name(qiv, name), "number");
}
Let's clean this up to the more common
if (!qnum) {
error_setg(errp, QERR_INVALID_PARAMETER_TYPE,
full_name(qiv, name), "number");
return;
}
*obj = qnum_get_double(qnum);
Preferrably in another patch, though.
> diff --git a/qapi/qobject-output-visitor.c b/qapi/qobject-output-visitor.c
> index 871127079d..2ca5093b22 100644
> --- a/qapi/qobject-output-visitor.c
> +++ b/qapi/qobject-output-visitor.c
> @@ -144,7 +144,7 @@ static void qobject_output_type_int64(Visitor *v, const
> char *name,
> int64_t *obj, Error **errp)
> {
> QObjectOutputVisitor *qov = to_qov(v);
> - qobject_output_add(qov, name, qint_from_int(*obj));
> + qobject_output_add(qov, name, qnum_from_int(*obj));
> }
>
> static void qobject_output_type_uint64(Visitor *v, const char *name,
> @@ -152,7 +152,7 @@ static void qobject_output_type_uint64(Visitor *v, const
> char *name,
> {
> /* FIXME values larger than INT64_MAX become negative */
> QObjectOutputVisitor *qov = to_qov(v);
> - qobject_output_add(qov, name, qint_from_int(*obj));
> + qobject_output_add(qov, name, qnum_from_int(*obj));
> }
>
> static void qobject_output_type_bool(Visitor *v, const char *name, bool *obj,
> @@ -177,7 +177,7 @@ static void qobject_output_type_number(Visitor *v, const
> char *name,
> double *obj, Error **errp)
> {
> QObjectOutputVisitor *qov = to_qov(v);
> - qobject_output_add(qov, name, qfloat_from_double(*obj));
> + qobject_output_add(qov, name, qnum_from_double(*obj));
> }
>
> static void qobject_output_type_any(Visitor *v, const char *name,
> diff --git a/qga/commands.c b/qga/commands.c
> index 3333ed50b2..ff89e805cf 100644
> --- a/qga/commands.c
> +++ b/qga/commands.c
> @@ -485,7 +485,7 @@ int ga_parse_whence(GuestFileWhence *whence, Error **errp)
> {
> /* Exploit the fact that we picked values to match QGA_SEEK_*. */
> if (whence->type == QTYPE_QSTRING) {
> - whence->type = QTYPE_QINT;
> + whence->type = QTYPE_QNUM;
> whence->u.value = whence->u.name;
> }
> switch (whence->u.value) {
> diff --git a/qga/main.c b/qga/main.c
> index cc58d2b53d..405c1290f8 100644
> --- a/qga/main.c
> +++ b/qga/main.c
> @@ -19,7 +19,6 @@
> #endif
> #include "qapi/qmp/json-streamer.h"
> #include "qapi/qmp/json-parser.h"
> -#include "qapi/qmp/qint.h"
> #include "qapi/qmp/qjson.h"
> #include "qga/guest-agent-core.h"
> #include "qemu/module.h"
> diff --git a/qobject/json-parser.c b/qobject/json-parser.c
> index c18e48ab94..b90b2fb45a 100644
> --- a/qobject/json-parser.c
> +++ b/qobject/json-parser.c
> @@ -466,16 +466,16 @@ static QObject *parse_escape(JSONParserContext *ctxt,
> va_list *ap)
> } else if (!strcmp(token->str, "%i")) {
> return QOBJECT(qbool_from_bool(va_arg(*ap, int)));
> } else if (!strcmp(token->str, "%d")) {
> - return QOBJECT(qint_from_int(va_arg(*ap, int)));
> + return QOBJECT(qnum_from_int(va_arg(*ap, int)));
> } else if (!strcmp(token->str, "%ld")) {
> - return QOBJECT(qint_from_int(va_arg(*ap, long)));
> + return QOBJECT(qnum_from_int(va_arg(*ap, long)));
> } else if (!strcmp(token->str, "%lld") ||
> !strcmp(token->str, "%I64d")) {
> - return QOBJECT(qint_from_int(va_arg(*ap, long long)));
> + return QOBJECT(qnum_from_int(va_arg(*ap, long long)));
> } else if (!strcmp(token->str, "%s")) {
> return QOBJECT(qstring_from_str(va_arg(*ap, const char *)));
> } else if (!strcmp(token->str, "%f")) {
> - return QOBJECT(qfloat_from_double(va_arg(*ap, double)));
> + return QOBJECT(qnum_from_double(va_arg(*ap, double)));
> }
> return NULL;
> }
> @@ -491,24 +491,21 @@ static QObject *parse_literal(JSONParserContext *ctxt)
> case JSON_STRING:
> return QOBJECT(qstring_from_escaped_str(ctxt, token));
> case JSON_INTEGER: {
> - /* A possibility exists that this is a whole-valued float where the
> - * fractional part was left out due to being 0 (.0). It's not a big
> - * deal to treat these as ints in the parser, so long as users of the
> - * resulting QObject know to expect a QInt in place of a QFloat in
> - * cases like these.
> + /*
> + * Represent JSON_INTEGER as QNUM_I64 if possible, else as
> + * QNUM_DOUBLE. Note that strtoll() fails with ERANGE when
Two spaces after the period, for consistency with existing comments
around here. Hmm, you fix it in PATCH 13.
> + * it's not possible.
> *
> - * However, in some cases these values will overflow/underflow a
> - * QInt/int64 container, thus we should assume these are to be
> handled
> - * as QFloats/doubles rather than silently changing their values.
> - *
> - * strtoll() indicates these instances by setting errno to ERANGE
> + * qnum_get_int() will then work for any signed 64-bit
> + * JSON_INTEGER, and qnum_get_double both for any JSON_INTEGER
qnum_get_double()
> + * and any JSON_FLOAT.
> */
Worth mentioning that qnum_get_double() loses precision for integers
beyond 53 bits?
> int64_t value;
>
> errno = 0; /* strtoll doesn't set errno on success */
> value = strtoll(token->str, NULL, 10);
> if (errno != ERANGE) {
> - return QOBJECT(qint_from_int(value));
> + return QOBJECT(qnum_from_int(value));
> }
> /* fall through to JSON_FLOAT */
> }
> @@ -516,7 +513,7 @@ static QObject *parse_literal(JSONParserContext *ctxt)
> /* FIXME dependent on locale; a pervasive issue in QEMU */
> /* FIXME our lexer matches RFC 7159 in forbidding Inf or NaN,
> * but those might be useful extensions beyond JSON */
> - return QOBJECT(qfloat_from_double(strtod(token->str, NULL)));
> + return QOBJECT(qnum_from_double(strtod(token->str, NULL)));
> default:
> abort();
> }
> diff --git a/qobject/qdict.c b/qobject/qdict.c
> index 88e2ecd658..f62625cbd6 100644
> --- a/qobject/qdict.c
> +++ b/qobject/qdict.c
> @@ -11,8 +11,7 @@
> */
>
> #include "qemu/osdep.h"
> -#include "qapi/qmp/qint.h"
> -#include "qapi/qmp/qfloat.h"
> +#include "qapi/qmp/qnum.h"
> #include "qapi/qmp/qdict.h"
> #include "qapi/qmp/qbool.h"
> #include "qapi/qmp/qstring.h"
> @@ -180,37 +179,32 @@ size_t qdict_size(const QDict *qdict)
> /**
> * qdict_get_double(): Get an number mapped by 'key'
> *
> - * This function assumes that 'key' exists and it stores a
> - * QFloat or QInt object.
> + * This function assumes that 'key' exists and it stores a QNum.
> *
> * Return number mapped by 'key'.
> */
> double qdict_get_double(const QDict *qdict, const char *key)
> {
> - QObject *obj = qdict_get(qdict, key);
> -
> - assert(obj);
> - switch (qobject_type(obj)) {
> - case QTYPE_QFLOAT:
> - return qfloat_get_double(qobject_to_qfloat(obj));
> - case QTYPE_QINT:
> - return qint_get_int(qobject_to_qint(obj));
> - default:
> - abort();
> - }
> + return qnum_get_double(qobject_to_qnum(qdict_get(qdict, key)));
> }
>
> /**
> * qdict_get_int(): Get an integer mapped by 'key'
> *
> * This function assumes that 'key' exists and it stores a
> - * QInt object.
> + * QNum representable as int.
> *
> * Return integer mapped by 'key'.
> */
> int64_t qdict_get_int(const QDict *qdict, const char *key)
> {
> - return qint_get_int(qobject_to_qint(qdict_get(qdict, key)));
> + int64_t val;
> +
> + if (!qnum_get_int(qobject_to_qnum(qdict_get(qdict, key)), &val)) {
> + g_assert_not_reached();
> + }
> +
> + return val;
> }
>
> /**
> @@ -259,16 +253,21 @@ const char *qdict_get_str(const QDict *qdict, const
> char *key)
> /**
> * qdict_get_try_int(): Try to get integer mapped by 'key'
> *
> - * Return integer mapped by 'key', if it is not present in
> - * the dictionary or if the stored object is not of QInt type
> - * 'def_value' will be returned.
> + * Return integer mapped by 'key', if it is not present in the
> + * dictionary or if the stored object is not a QNum representing an
> + * integer, 'def_value' will be returned.
> */
> int64_t qdict_get_try_int(const QDict *qdict, const char *key,
> int64_t def_value)
> {
> - QInt *qint = qobject_to_qint(qdict_get(qdict, key));
> + QNum *qnum = qobject_to_qnum(qdict_get(qdict, key));
> + int64_t val;
> +
> + if (!qnum || !qnum_get_int(qnum, &val)) {
> + return def_value;
> + }
>
> - return qint ? qint_get_int(qint) : def_value;
> + return val;
> }
Let me copy a remark from the previous review I'd like to remember going
forward. I don't expect you to do anything about it right now.
The function does the right thing for QNums created by the JSON parser,
which picks the QNumType carefully, in parse_literal():
* if the JSON number has neither a fractional nor an exponent part, and
- fits into int64_t, use qnum_from_int()
- fits into uint64_t, use qnum_from_uint()
(if it fits both, qnum_from_int(), but that should not matter)
* else, use qnum_from_double().
For such a QNum, rejecting QNUM_DOUBLE here is correct, because the
value either had a fractional part, an exponent part, or didn't fit
int64_t.
Other code using qnum_from_int() and qnum_from_double() needs to be
similarly careful.
>
> /**
> diff --git a/qobject/qfloat.c b/qobject/qfloat.c
> deleted file mode 100644
> index d5da847701..0000000000
> --- a/qobject/qfloat.c
> +++ /dev/null
> @@ -1,62 +0,0 @@
> -/*
> - * QFloat Module
> - *
> - * Copyright IBM, Corp. 2009
> - *
> - * Authors:
> - * Anthony Liguori <address@hidden>
> - *
> - * This work is licensed under the terms of the GNU LGPL, version 2.1 or
> later.
> - * See the COPYING.LIB file in the top-level directory.
> - *
> - */
> -
> -#include "qemu/osdep.h"
> -#include "qapi/qmp/qfloat.h"
> -#include "qapi/qmp/qobject.h"
> -#include "qemu-common.h"
> -
> -/**
> - * qfloat_from_int(): Create a new QFloat from a float
> - *
> - * Return strong reference.
> - */
> -QFloat *qfloat_from_double(double value)
> -{
> - QFloat *qf;
> -
> - qf = g_malloc(sizeof(*qf));
> - qobject_init(QOBJECT(qf), QTYPE_QFLOAT);
> - qf->value = value;
> -
> - return qf;
> -}
> -
> -/**
> - * qfloat_get_double(): Get the stored float
> - */
> -double qfloat_get_double(const QFloat *qf)
> -{
> - return qf->value;
> -}
> -
> -/**
> - * qobject_to_qfloat(): Convert a QObject into a QFloat
> - */
> -QFloat *qobject_to_qfloat(const QObject *obj)
> -{
> - if (!obj || qobject_type(obj) != QTYPE_QFLOAT) {
> - return NULL;
> - }
> - return container_of(obj, QFloat, base);
> -}
> -
> -/**
> - * qfloat_destroy_obj(): Free all memory allocated by a
> - * QFloat object
> - */
> -void qfloat_destroy_obj(QObject *obj)
> -{
> - assert(obj != NULL);
> - g_free(qobject_to_qfloat(obj));
> -}
> diff --git a/qobject/qint.c b/qobject/qint.c
> deleted file mode 100644
> index d7d1b3021f..0000000000
> --- a/qobject/qint.c
> +++ /dev/null
> @@ -1,61 +0,0 @@
> -/*
> - * QInt Module
> - *
> - * Copyright (C) 2009 Red Hat Inc.
> - *
> - * Authors:
> - * Luiz Capitulino <address@hidden>
> - *
> - * This work is licensed under the terms of the GNU LGPL, version 2.1 or
> later.
> - * See the COPYING.LIB file in the top-level directory.
> - */
> -
> -#include "qemu/osdep.h"
> -#include "qapi/qmp/qint.h"
> -#include "qapi/qmp/qobject.h"
> -#include "qemu-common.h"
> -
> -/**
> - * qint_from_int(): Create a new QInt from an int64_t
> - *
> - * Return strong reference.
> - */
> -QInt *qint_from_int(int64_t value)
> -{
> - QInt *qi;
> -
> - qi = g_malloc(sizeof(*qi));
> - qobject_init(QOBJECT(qi), QTYPE_QINT);
> - qi->value = value;
> -
> - return qi;
> -}
> -
> -/**
> - * qint_get_int(): Get the stored integer
> - */
> -int64_t qint_get_int(const QInt *qi)
> -{
> - return qi->value;
> -}
> -
> -/**
> - * qobject_to_qint(): Convert a QObject into a QInt
> - */
> -QInt *qobject_to_qint(const QObject *obj)
> -{
> - if (!obj || qobject_type(obj) != QTYPE_QINT) {
> - return NULL;
> - }
> - return container_of(obj, QInt, base);
> -}
> -
> -/**
> - * qint_destroy_obj(): Free all memory allocated by a
> - * QInt object
> - */
> -void qint_destroy_obj(QObject *obj)
> -{
> - assert(obj != NULL);
> - g_free(qobject_to_qint(obj));
> -}
> diff --git a/qobject/qjson.c b/qobject/qjson.c
> index b2f3bfec53..2e0930884e 100644
> --- a/qobject/qjson.c
> +++ b/qobject/qjson.c
> @@ -132,12 +132,11 @@ static void to_json(const QObject *obj, QString *str,
> int pretty, int indent)
> case QTYPE_QNULL:
> qstring_append(str, "null");
> break;
> - case QTYPE_QINT: {
> - QInt *val = qobject_to_qint(obj);
> - char buffer[1024];
> -
> - snprintf(buffer, sizeof(buffer), "%" PRId64, qint_get_int(val));
> + case QTYPE_QNUM: {
> + QNum *val = qobject_to_qnum(obj);
> + char *buffer = qnum_to_string(val);
> qstring_append(str, buffer);
> + g_free(buffer);
> break;
> }
> case QTYPE_QSTRING: {
> @@ -234,34 +233,6 @@ static void to_json(const QObject *obj, QString *str,
> int pretty, int indent)
> qstring_append(str, "]");
> break;
> }
> - case QTYPE_QFLOAT: {
> - QFloat *val = qobject_to_qfloat(obj);
> - char buffer[1024];
> - int len;
> -
> - /* FIXME: snprintf() is locale dependent; but JSON requires
> - * numbers to be formatted as if in the C locale. Dependence
> - * on C locale is a pervasive issue in QEMU. */
> - /* FIXME: This risks printing Inf or NaN, which are not valid
> - * JSON values. */
> - /* FIXME: the default precision of 6 for %f often causes
> - * 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. */
> - len = snprintf(buffer, sizeof(buffer), "%f", qfloat_get_double(val));
> - while (len > 0 && buffer[len - 1] == '0') {
> - len--;
> - }
> -
> - if (len && buffer[len - 1] == '.') {
> - buffer[len - 1] = 0;
> - } else {
> - buffer[len] = 0;
> - }
> -
> - qstring_append(str, buffer);
> - break;
> - }
> case QTYPE_QBOOL: {
> QBool *val = qobject_to_qbool(obj);
>
> diff --git a/qobject/qnum.c b/qobject/qnum.c
> new file mode 100644
> index 0000000000..f16864160a
> --- /dev/null
> +++ b/qobject/qnum.c
> @@ -0,0 +1,141 @@
> +/*
> + * QNum Module
> + *
> + * Copyright (C) 2009 Red Hat Inc.
> + *
> + * Authors:
> + * Luiz Capitulino <address@hidden>
> + * Marc-André Lureau <address@hidden>
> + *
> + * This work is licensed under the terms of the GNU LGPL, version 2.1 or
> later.
> + * See the COPYING.LIB file in the top-level directory.
> + */
> +
> +#include "qemu/osdep.h"
> +#include "qapi/error.h"
> +#include "qapi/qmp/qnum.h"
> +#include "qapi/qmp/qobject.h"
> +#include "qemu-common.h"
> +
> +/**
> + * qnum_from_int(): Create a new QNum from an int64_t
> + *
> + * Return strong reference.
> + */
> +QNum *qnum_from_int(int64_t value)
> +{
> + QNum *qn = g_new(QNum, 1);
> +
> + qobject_init(QOBJECT(qn), QTYPE_QNUM);
> + qn->kind = QNUM_I64;
> + qn->u.i64 = value;
> +
> + return qn;
> +}
> +
> +/**
> + * qnum_from_double(): Create a new QNum from a double
> + *
> + * Return strong reference.
> + */
> +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;
> +}
> +
> +/**
> + * qnum_get_int(): Get an integer representation of the number
> + *
> + * Return true on success.
> + */
> +bool qnum_get_int(const QNum *qn, int64_t *val)
> +{
> + switch (qn->kind) {
> + case QNUM_I64:
> + *val = qn->u.i64;
> + return true;
> + case QNUM_DOUBLE:
> + return false;
> + }
> +
> + g_assert_not_reached();
Please stick to plain assert() in qapi/ for now.
> + return false;
> +}
> +
> +/**
> + * qnum_get_double(): Get a float representation of the number
> + */
> +double qnum_get_double(QNum *qn)
> +{
> + switch (qn->kind) {
> + case QNUM_I64:
> + return qn->u.i64;
> + case QNUM_DOUBLE:
> + return qn->u.dbl;
> + }
> +
> + g_assert_not_reached();
> +}
> +
> +char *qnum_to_string(QNum *qn)
> +{
> + char *buffer;
> + int len;
> +
> + switch (qn->kind) {
> + case QNUM_I64:
> + return g_strdup_printf("%" PRId64, qn->u.i64);
> + case QNUM_DOUBLE:
> + /* FIXME: snprintf() is locale dependent; but JSON requires
> + * numbers to be formatted as if in the C locale. Dependence
> + * on C locale is a pervasive issue in QEMU. */
> + /* FIXME: This risks printing Inf or NaN, which are not valid
> + * JSON values. */
> + /* FIXME: the default precision of 6 for %f often causes
> + * 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);
> + len = strlen(buffer);
> + while (len > 0 && buffer[len - 1] == '0') {
> + len--;
> + }
> +
> + if (len && buffer[len - 1] == '.') {
> + buffer[len - 1] = 0;
> + } else {
> + buffer[len] = 0;
> + }
> +
> + return buffer;
> + }
> +
> + g_assert_not_reached();
> +}
> +
> +/**
> + * qobject_to_qnum(): Convert a QObject into a QNum
> + */
> +QNum *qobject_to_qnum(const QObject *obj)
> +{
> + if (!obj || qobject_type(obj) != QTYPE_QNUM) {
> + return NULL;
> + }
> + return container_of(obj, QNum, base);
> +}
> +
> +/**
> + * qnum_destroy_obj(): Free all memory allocated by a
> + * QNum object
> + */
> +void qnum_destroy_obj(QObject *obj)
> +{
> + assert(obj != NULL);
> + g_free(qobject_to_qnum(obj));
> +}
> diff --git a/qobject/qobject.c b/qobject/qobject.c
> index fe4fa10989..b0cafb66f1 100644
> --- a/qobject/qobject.c
> +++ b/qobject/qobject.c
> @@ -14,11 +14,10 @@
> static void (*qdestroy[QTYPE__MAX])(QObject *) = {
> [QTYPE_NONE] = NULL, /* No such object exists */
> [QTYPE_QNULL] = NULL, /* qnull_ is indestructible */
> - [QTYPE_QINT] = qint_destroy_obj,
> + [QTYPE_QNUM] = qnum_destroy_obj,
> [QTYPE_QSTRING] = qstring_destroy_obj,
> [QTYPE_QDICT] = qdict_destroy_obj,
> [QTYPE_QLIST] = qlist_destroy_obj,
> - [QTYPE_QFLOAT] = qfloat_destroy_obj,
> [QTYPE_QBOOL] = qbool_destroy_obj,
> };
>
> diff --git a/qom/object.c b/qom/object.c
> index c7b8079df6..cf0e64a625 100644
> --- a/qom/object.c
> +++ b/qom/object.c
> @@ -27,7 +27,6 @@
> #include "qom/qom-qobject.h"
> #include "qapi/qmp/qobject.h"
> #include "qapi/qmp/qbool.h"
> -#include "qapi/qmp/qint.h"
> #include "qapi/qmp/qstring.h"
>
> #define MAX_INTERFACES 32
> @@ -1190,28 +1189,27 @@ bool object_property_get_bool(Object *obj, const char
> *name,
> void object_property_set_int(Object *obj, int64_t value,
> const char *name, Error **errp)
> {
> - QInt *qint = qint_from_int(value);
> - object_property_set_qobject(obj, QOBJECT(qint), name, errp);
> + QNum *qnum = qnum_from_int(value);
> + object_property_set_qobject(obj, QOBJECT(qnum), name, errp);
>
> - QDECREF(qint);
> + QDECREF(qnum);
> }
>
> int64_t object_property_get_int(Object *obj, const char *name,
> Error **errp)
> {
> QObject *ret = object_property_get_qobject(obj, name, errp);
> - QInt *qint;
> + QNum *qnum;
> int64_t retval;
>
> if (!ret) {
> return -1;
> }
> - qint = qobject_to_qint(ret);
> - if (!qint) {
> +
> + qnum = qobject_to_qnum(ret);
> + if (!qnum || !qnum_get_int(qnum, &retval)) {
> error_setg(errp, QERR_INVALID_PARAMETER_TYPE, name, "int");
> retval = -1;
> - } else {
> - retval = qint_get_int(qint);
> }
>
> qobject_decref(ret);
> diff --git a/target/i386/cpu.c b/target/i386/cpu.c
> index a41d595c23..12882f0774 100644
> --- a/target/i386/cpu.c
> +++ b/target/i386/cpu.c
> @@ -29,11 +29,7 @@
> #include "qemu/option.h"
> #include "qemu/config-file.h"
> #include "qapi/qmp/qerror.h"
> -#include "qapi/qmp/qstring.h"
> -#include "qapi/qmp/qdict.h"
> -#include "qapi/qmp/qbool.h"
> -#include "qapi/qmp/qint.h"
> -#include "qapi/qmp/qfloat.h"
> +#include "qapi/qmp/types.h"
>
> #include "qapi-types.h"
> #include "qapi-visit.h"
You seem to have missed the second part of my review,
Message-ID: <address@hidden>. No big deal, I simply
copy the comments that still apply.
> diff --git a/tests/check-qdict.c b/tests/check-qdict.c
> index be8d81f07b..5c70490e83 100644
> --- a/tests/check-qdict.c
> +++ b/tests/check-qdict.c
> @@ -11,7 +11,6 @@
> */
> #include "qemu/osdep.h"
>
> -#include "qapi/qmp/qint.h"
> #include "qapi/qmp/qdict.h"
> #include "qapi/qmp/qstring.h"
> #include "qapi/error.h"
> @@ -39,10 +38,11 @@ static void qdict_new_test(void)
>
> static void qdict_put_obj_test(void)
> {
> - QInt *qi;
> + QNum *qi;
> QDict *qdict;
> QDictEntry *ent;
> const int num = 42;
> + int64_t val;
>
> qdict = qdict_new();
>
> @@ -51,8 +51,9 @@ static void qdict_put_obj_test(void)
>
> g_assert(qdict_size(qdict) == 1);
> ent = QLIST_FIRST(&qdict->table[12345 % QDICT_BUCKET_MAX]);
> - qi = qobject_to_qint(ent->value);
> - g_assert(qint_get_int(qi) == num);
> + qi = qobject_to_qnum(ent->value);
> + g_assert(qnum_get_int(qi, &val));
> + g_assert_cmpint(val, ==, num);
This is a variation of the pattern I noted above. It occurs many, many
times. I believe the proposed helper would ease review.
>
> // destroy doesn't exit yet
> QDECREF(qi);
Since you're touching three out of four lines containing @qi anyway:
rename it to @qn? You rename like that in some places, but not all.
> @@ -74,19 +75,21 @@ static void qdict_destroy_simple_test(void)
>
> static void qdict_get_test(void)
> {
> - QInt *qi;
> + QNum *qi;
> QObject *obj;
> const int value = -42;
> const char *key = "test";
> QDict *tests_dict = qdict_new();
> + int64_t val;
>
> qdict_put_int(tests_dict, key, value);
>
> obj = qdict_get(tests_dict, key);
> g_assert(obj != NULL);
>
> - qi = qobject_to_qint(obj);
> - g_assert(qint_get_int(qi) == value);
> + qi = qobject_to_qnum(obj);
> + g_assert(qnum_get_int(qi, &val));
> + g_assert_cmpint(val, ==, value);
>
> QDECREF(tests_dict);
> }
> @@ -329,8 +332,9 @@ static void qdict_array_split_test(void)
> {
> QDict *test_dict = qdict_new();
> QDict *dict1, *dict2;
> - QInt *int1;
> + QNum *int1;
> QList *test_list;
> + int64_t val;
>
> /*
> * Test the split of
> @@ -380,7 +384,7 @@ static void qdict_array_split_test(void)
>
> dict1 = qobject_to_qdict(qlist_pop(test_list));
> dict2 = qobject_to_qdict(qlist_pop(test_list));
> - int1 = qobject_to_qint(qlist_pop(test_list));
> + int1 = qobject_to_qnum(qlist_pop(test_list));
>
> g_assert(dict1);
> g_assert(dict2);
> @@ -402,7 +406,8 @@ static void qdict_array_split_test(void)
>
> QDECREF(dict2);
>
> - g_assert(qint_get_int(int1) == 66);
> + g_assert(qnum_get_int(int1, &val));
> + g_assert_cmpint(val, ==, 66);
>
> QDECREF(int1);
>
> @@ -447,14 +452,15 @@ static void qdict_array_split_test(void)
>
> qdict_array_split(test_dict, &test_list);
>
> - int1 = qobject_to_qint(qlist_pop(test_list));
> + int1 = qobject_to_qnum(qlist_pop(test_list));
>
> g_assert(int1);
> g_assert(qlist_empty(test_list));
>
> QDECREF(test_list);
>
> - g_assert(qint_get_int(int1) == 42);
> + g_assert(qnum_get_int(int1, &val));
> + g_assert_cmpint(val, ==, 42);
>
> QDECREF(int1);
>
> diff --git a/tests/check-qfloat.c b/tests/check-qfloat.c
> deleted file mode 100644
> index 1da2cdae08..0000000000
> --- a/tests/check-qfloat.c
> +++ /dev/null
> @@ -1,53 +0,0 @@
> -/*
> - * QFloat unit-tests.
> - *
> - * Copyright IBM, Corp. 2009
> - *
> - * Authors:
> - * Anthony Liguori <address@hidden>
> - *
> - * This work is licensed under the terms of the GNU LGPL, version 2.1 or
> later.
> - * See the COPYING.LIB file in the top-level directory.
> - *
> - */
> -#include "qemu/osdep.h"
> -
> -#include "qapi/qmp/qfloat.h"
> -#include "qemu-common.h"
> -
> -/*
> - * Public Interface test-cases
> - *
> - * (with some violations to access 'private' data)
> - */
> -
> -static void qfloat_from_double_test(void)
> -{
> - QFloat *qf;
> - const double value = -42.23423;
> -
> - qf = qfloat_from_double(value);
> - g_assert(qf != NULL);
> - g_assert(qf->value == value);
> - g_assert(qf->base.refcnt == 1);
> - g_assert(qobject_type(QOBJECT(qf)) == QTYPE_QFLOAT);
> -
> - // destroy doesn't exit yet
> - g_free(qf);
> -}
> -
> -static void qfloat_destroy_test(void)
> -{
> - QFloat *qf = qfloat_from_double(0.0);
> - QDECREF(qf);
> -}
> -
> -int main(int argc, char **argv)
> -{
> - g_test_init(&argc, &argv, NULL);
> -
> - g_test_add_func("/public/from_double", qfloat_from_double_test);
> - g_test_add_func("/public/destroy", qfloat_destroy_test);
> -
> - return g_test_run();
> -}
> diff --git a/tests/check-qint.c b/tests/check-qint.c
> deleted file mode 100644
> index b6e4555115..0000000000
> --- a/tests/check-qint.c
> +++ /dev/null
> @@ -1,87 +0,0 @@
> -/*
> - * QInt unit-tests.
> - *
> - * Copyright (C) 2009 Red Hat Inc.
> - *
> - * Authors:
> - * Luiz Capitulino <address@hidden>
> - *
> - * This work is licensed under the terms of the GNU LGPL, version 2.1 or
> later.
> - * See the COPYING.LIB file in the top-level directory.
> - */
> -#include "qemu/osdep.h"
> -
> -#include "qapi/qmp/qint.h"
> -#include "qemu-common.h"
> -
> -/*
> - * Public Interface test-cases
> - *
> - * (with some violations to access 'private' data)
> - */
> -
> -static void qint_from_int_test(void)
> -{
> - QInt *qi;
> - const int value = -42;
> -
> - qi = qint_from_int(value);
> - g_assert(qi != NULL);
> - g_assert(qi->value == value);
> - g_assert(qi->base.refcnt == 1);
> - g_assert(qobject_type(QOBJECT(qi)) == QTYPE_QINT);
> -
> - // destroy doesn't exit yet
> - g_free(qi);
> -}
> -
> -static void qint_destroy_test(void)
> -{
> - QInt *qi = qint_from_int(0);
> - QDECREF(qi);
> -}
> -
> -static void qint_from_int64_test(void)
> -{
> - QInt *qi;
> - const int64_t value = 0x1234567890abcdefLL;
> -
> - qi = qint_from_int(value);
> - g_assert((int64_t) qi->value == value);
> -
> - QDECREF(qi);
> -}
> -
> -static void qint_get_int_test(void)
> -{
> - QInt *qi;
> - const int value = 123456;
> -
> - qi = qint_from_int(value);
> - g_assert(qint_get_int(qi) == value);
> -
> - QDECREF(qi);
> -}
> -
> -static void qobject_to_qint_test(void)
> -{
> - QInt *qi;
> -
> - qi = qint_from_int(0);
> - g_assert(qobject_to_qint(QOBJECT(qi)) == qi);
> -
> - QDECREF(qi);
> -}
> -
> -int main(int argc, char **argv)
> -{
> - g_test_init(&argc, &argv, NULL);
> -
> - g_test_add_func("/public/from_int", qint_from_int_test);
> - g_test_add_func("/public/destroy", qint_destroy_test);
> - g_test_add_func("/public/from_int64", qint_from_int64_test);
> - g_test_add_func("/public/get_int", qint_get_int_test);
> - g_test_add_func("/public/to_qint", qobject_to_qint_test);
> -
> - return g_test_run();
> -}
> diff --git a/tests/check-qjson.c b/tests/check-qjson.c
> index 963dd46f07..8ec728a702 100644
> --- a/tests/check-qjson.c
> +++ b/tests/check-qjson.c
> @@ -886,21 +886,23 @@ static void simple_number(void)
> };
>
> for (i = 0; test_cases[i].encoded; i++) {
> - QInt *qint;
> + QNum *qnum;
> + int64_t val;
>
> - qint = qobject_to_qint(qobject_from_json(test_cases[i].encoded,
> + qnum = qobject_to_qnum(qobject_from_json(test_cases[i].encoded,
> &error_abort));
> - g_assert(qint);
> - g_assert(qint_get_int(qint) == test_cases[i].decoded);
> + g_assert(qnum);
> + g_assert(qnum_get_int(qnum, &val));
> + g_assert_cmpint(val, ==, test_cases[i].decoded);
> if (test_cases[i].skip == 0) {
> QString *str;
>
> - str = qobject_to_json(QOBJECT(qint));
> + str = qobject_to_json(QOBJECT(qnum));
> g_assert(strcmp(qstring_get_str(str), test_cases[i].encoded) ==
> 0);
> QDECREF(str);
> }
>
> - QDECREF(qint);
> + QDECREF(qnum);
> }
> }
>
> @@ -921,12 +923,12 @@ static void float_number(void)
>
> for (i = 0; test_cases[i].encoded; i++) {
> QObject *obj;
> - QFloat *qfloat;
> + QNum *qnum;
>
> obj = qobject_from_json(test_cases[i].encoded, &error_abort);
> - qfloat = qobject_to_qfloat(obj);
> - g_assert(qfloat);
> - g_assert(qfloat_get_double(qfloat) == test_cases[i].decoded);
> + qnum = qobject_to_qnum(obj);
> + g_assert(qnum);
> + g_assert(qnum_get_double(qnum) == test_cases[i].decoded);
>
> if (test_cases[i].skip == 0) {
> QString *str;
> @@ -936,29 +938,31 @@ static void float_number(void)
> QDECREF(str);
> }
>
> - QDECREF(qfloat);
> + QDECREF(qnum);
> }
> }
>
> static void vararg_number(void)
> {
> - QInt *qint;
> - QFloat *qfloat;
> + QNum *qnum;
> int value = 0x2342;
> long long value_ll = 0x2342342343LL;
> double valuef = 2.323423423;
> + int64_t val;
>
> - qint = qobject_to_qint(qobject_from_jsonf("%d", value));
> - g_assert(qint_get_int(qint) == value);
> - QDECREF(qint);
> + qnum = qobject_to_qnum(qobject_from_jsonf("%d", value));
> + g_assert(qnum_get_int(qnum, &val));
> + g_assert_cmpint(val, ==, value);
> + QDECREF(qnum);
>
> - qint = qobject_to_qint(qobject_from_jsonf("%lld", value_ll));
> - g_assert(qint_get_int(qint) == value_ll);
> - QDECREF(qint);
> + qnum = qobject_to_qnum(qobject_from_jsonf("%lld", value_ll));
> + g_assert(qnum_get_int(qnum, &val));
> + g_assert_cmpint(val, ==, value_ll);
> + QDECREF(qnum);
>
> - qfloat = qobject_to_qfloat(qobject_from_jsonf("%f", valuef));
> - g_assert(qfloat_get_double(qfloat) == valuef);
> - QDECREF(qfloat);
> + qnum = qobject_to_qnum(qobject_from_jsonf("%f", valuef));
> + g_assert(qnum_get_double(qnum) == valuef);
> + QDECREF(qnum);
> }
>
> static void keyword_literal(void)
> @@ -1019,7 +1023,7 @@ struct LiteralQObject
> {
> int type;
> union {
> - int64_t qint;
> + int64_t qnum;
> const char *qstr;
> LiteralQDictEntry *qdict;
> LiteralQObject *qlist;
> @@ -1032,7 +1036,7 @@ struct LiteralQDictEntry
> LiteralQObject value;
> };
>
> -#define QLIT_QINT(val) (LiteralQObject){.type = QTYPE_QINT, .value.qint =
> (val)}
> +#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)}
Aside: yet another private way to compare actual QObjects to expected
ones. We should pick *one* way to compare, and stick to it.
> @@ -1064,13 +1068,16 @@ static void compare_helper(QObject *obj, void *opaque)
>
> 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_QINT:
> - return lhs->value.qint == qint_get_int(qobject_to_qint(rhs));
> + case QTYPE_QNUM:
> + g_assert(qnum_get_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: {
> @@ -1114,7 +1121,7 @@ static void simple_dict(void)
> {
> .encoded = "{\"foo\": 42, \"bar\": \"hello world\"}",
> .decoded = QLIT_QDICT(((LiteralQDictEntry[]){
> - { "foo", QLIT_QINT(42) },
> + { "foo", QLIT_QNUM(42) },
> { "bar", QLIT_QSTR("hello world") },
> { }
> })),
> @@ -1126,7 +1133,7 @@ static void simple_dict(void)
> }, {
> .encoded = "{\"foo\": 43}",
> .decoded = QLIT_QDICT(((LiteralQDictEntry[]){
> - { "foo", QLIT_QINT(43) },
> + { "foo", QLIT_QNUM(43) },
> { }
> })),
> },
> @@ -1212,15 +1219,15 @@ static void simple_list(void)
> {
> .encoded = "[43,42]",
> .decoded = QLIT_QLIST(((LiteralQObject[]){
> - QLIT_QINT(43),
> - QLIT_QINT(42),
> + QLIT_QNUM(43),
> + QLIT_QNUM(42),
> { }
> })),
> },
> {
> .encoded = "[43]",
> .decoded = QLIT_QLIST(((LiteralQObject[]){
> - QLIT_QINT(43),
> + QLIT_QNUM(43),
> { }
> })),
> },
> @@ -1269,35 +1276,35 @@ static void simple_whitespace(void)
> {
> .encoded = " [ 43 , 42 ]",
> .decoded = QLIT_QLIST(((LiteralQObject[]){
> - QLIT_QINT(43),
> - QLIT_QINT(42),
> + QLIT_QNUM(43),
> + QLIT_QNUM(42),
> { }
> })),
> },
> {
> .encoded = " [ 43 , { 'h' : 'b' }, [ ], 42 ]",
> .decoded = QLIT_QLIST(((LiteralQObject[]){
> - QLIT_QINT(43),
> + QLIT_QNUM(43),
> QLIT_QDICT(((LiteralQDictEntry[]){
> { "h", QLIT_QSTR("b") },
> { }})),
> QLIT_QLIST(((LiteralQObject[]){
> { }})),
> - QLIT_QINT(42),
> + QLIT_QNUM(42),
> { }
> })),
> },
> {
> .encoded = " [ 43 , { 'h' : 'b' , 'a' : 32 }, [ ], 42 ]",
> .decoded = QLIT_QLIST(((LiteralQObject[]){
> - QLIT_QINT(43),
> + QLIT_QNUM(43),
> QLIT_QDICT(((LiteralQDictEntry[]){
> { "h", QLIT_QSTR("b") },
> - { "a", QLIT_QINT(32) },
> + { "a", QLIT_QNUM(32) },
> { }})),
> QLIT_QLIST(((LiteralQObject[]){
> { }})),
> - QLIT_QINT(42),
> + QLIT_QNUM(42),
> { }
> })),
> },
> @@ -1327,11 +1334,11 @@ static void simple_varargs(void)
> QObject *embedded_obj;
> QObject *obj;
> LiteralQObject decoded = QLIT_QLIST(((LiteralQObject[]){
> - QLIT_QINT(1),
> - QLIT_QINT(2),
> + QLIT_QNUM(1),
> + QLIT_QNUM(2),
> QLIT_QLIST(((LiteralQObject[]){
> - QLIT_QINT(32),
> - QLIT_QINT(42),
> + QLIT_QNUM(32),
> + QLIT_QNUM(42),
> {}})),
> {}}));
>
> diff --git a/tests/check-qlist.c b/tests/check-qlist.c
> index 4983867c27..5bc8bb6756 100644
> --- a/tests/check-qlist.c
> +++ b/tests/check-qlist.c
> @@ -11,8 +11,8 @@
> */
> #include "qemu/osdep.h"
>
> -#include "qapi/qmp/qint.h"
> -#include "qapi/qmp/qlist.h"
> +#include "qapi/error.h"
> +#include "qapi/qmp/types.h"
qapi/qmp/types.h is a lazy way to increase compile times by including
more than you need. One day I'll kill it. Until then, I tolerate it in
.c, but not in .h. But I'd stick to just qlist.h and qnum.h here.
> /*
> * Public Interface test-cases
> @@ -35,11 +35,11 @@ static void qlist_new_test(void)
>
> static void qlist_append_test(void)
> {
> - QInt *qi;
> + QNum *qi;
> QList *qlist;
> QListEntry *entry;
>
> - qi = qint_from_int(42);
> + qi = qnum_from_int(42);
>
> qlist = qlist_new();
> qlist_append(qlist, qi);
> @@ -84,13 +84,17 @@ static const int iter_max = 42;
>
> static void iter_func(QObject *obj, void *opaque)
> {
> - QInt *qi;
> + QNum *qi;
> + int64_t val;
>
> g_assert(opaque == NULL);
>
> - qi = qobject_to_qint(obj);
> + qi = qobject_to_qnum(obj);
> g_assert(qi != NULL);
> - g_assert((qint_get_int(qi) >= 0) && (qint_get_int(qi) <= iter_max));
> +
> + g_assert(qnum_get_int(qi, &val));
> + g_assert_cmpint(val, >=, 0);
> + g_assert_cmpint(val, <=, iter_max);
>
> iter_called++;
> }
> diff --git a/tests/check-qnum.c b/tests/check-qnum.c
> new file mode 100644
> index 0000000000..4b3fec050e
> --- /dev/null
> +++ b/tests/check-qnum.c
Let's compare to the old check-qint.c and check-qfloat.c.
> @@ -0,0 +1,133 @@
> +/*
> + * QNum unit-tests.
> + *
> + * Copyright (C) 2009 Red Hat Inc.
Also merge check-qfloat.c's
* Copyright IBM, Corp. 2009
here, and
> + *
> + * Authors:
> + * Luiz Capitulino <address@hidden>
* Anthony Liguori <address@hidden>
here.
> + *
> + * 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.
> + */
Blank line here, please.
> +#include "qemu/osdep.h"
> +
> +#include "qapi/qmp/qnum.h"
> +#include "qapi/error.h"
> +#include "qemu-common.h"
> +
> +/*
> + * Public Interface test-cases
> + *
> + * (with some violations to access 'private' data)
> + */
I consider this comment close to useless.
> +
> +static void qnum_from_int_test(void)
> +{
> + QNum *qi;
> + const int value = -42;
> +
> + qi = qnum_from_int(value);
> + g_assert(qi != NULL);
g_assert_cmpint(qi->type, ==, QNUM_I64);
> + g_assert_cmpint(qi->u.i64, ==, value);
> + g_assert_cmpint(qi->base.refcnt, ==, 1);
> + g_assert_cmpint(qobject_type(QOBJECT(qi)), ==, QTYPE_QNUM);
> +
> + // destroy doesn't exit yet
> + g_free(qi);
> +}
> +
> +static void qnum_from_double_test(void)
> +{
> + QNum *qf;
> + const double value = -42.23423;
> +
> + qf = qnum_from_double(value);
> + g_assert(qf != NULL);
g_assert_cmpint(qf->type, ==, QNUM_DOUBLE);
> + g_assert_cmpfloat(qf->u.dbl, ==, value);
> + g_assert_cmpint(qf->base.refcnt, ==, 1);
> + g_assert_cmpint(qobject_type(QOBJECT(qf)), ==, QTYPE_QNUM);
> +
> + // destroy doesn't exit yet
> + g_free(qf);
> +}
Let's rename @qi and @qf to @qn.
> +
> +static void qnum_from_int64_test(void)
> +{
> + QNum *qi;
> + const int64_t value = 0x1234567890abcdefLL;
> +
> + qi = qnum_from_int(value);
> + g_assert_cmpint((int64_t) qi->u.i64, ==, value);
> +
> + QDECREF(qi);
> +}
> +
> +static void qnum_get_int_test(void)
> +{
> + QNum *qi;
> + const int value = 123456;
> + int64_t val;
> +
> + qi = qnum_from_int(value);
> + g_assert(qnum_get_int(qi, &val));
> + g_assert_cmpint(val, ==, value);
> +
> + QDECREF(qi);
> +}
> +
> +static void qobject_to_qnum_test(void)
> +{
> + QNum *qn;
> +
> + qn = qnum_from_int(0);
> + g_assert(qobject_to_qnum(QOBJECT(qn)) == qn);
> + QDECREF(qn);
> +
> + qn = qnum_from_double(0);
> + g_assert(qobject_to_qnum(QOBJECT(qn)) == qn);
> + QDECREF(qn);
> +}
You added this one. Makes sense, but announcing in the commit message
that you're also adding test cases wouldn't hurt.
> +
> +static void qnum_to_string_test(void)
> +{
> + QNum *qn;
> + char *tmp;
> +
> + qn = qnum_from_int(123456);
> + tmp = qnum_to_string(qn);
> + g_assert_cmpstr(tmp, ==, "123456");
> + g_free(tmp);
> + QDECREF(qn);
> +
> + qn = qnum_from_double(0.42);
> + tmp = qnum_to_string(qn);
> + g_assert_cmpstr(tmp, ==, "0.42");
> + g_free(tmp);
> + QDECREF(qn);
> +}
Also new. Good.
Test coverage could use further improvement, but this will do for now.
> +
> +static void qnum_destroy_test(void)
> +{
> + QNum *qn;
> +
> + qn = qnum_from_int(0);
> + QDECREF(qn);
> +
> + qn = qnum_from_double(0.42);
> + QDECREF(qn);
> +}
> +
> +int main(int argc, char **argv)
> +{
> + g_test_init(&argc, &argv, NULL);
> +
> + g_test_add_func("/qnum/from_int", qnum_from_int_test);
> + g_test_add_func("/qnum/from_double", qnum_from_double_test);
> + g_test_add_func("/qnum/destroy", qnum_destroy_test);
> + g_test_add_func("/qnum/from_int64", qnum_from_int64_test);
> + g_test_add_func("/qnum/get_int", qnum_get_int_test);
> + g_test_add_func("/qnum/to_qnum", qobject_to_qnum_test);
> + g_test_add_func("/qnum/to_string", qnum_to_string_test);
> +
> + return g_test_run();
> +}
> diff --git a/tests/test-qmp-commands.c b/tests/test-qmp-commands.c
> index acdded4d67..9c69c98781 100644
> --- a/tests/test-qmp-commands.c
> +++ b/tests/test-qmp-commands.c
> @@ -162,7 +162,8 @@ static void test_dispatch_cmd_io(void)
> QDict *ud1b = qdict_new();
> QDict *ret, *ret_dict, *ret_dict_dict, *ret_dict_dict_userdef;
> QDict *ret_dict_dict2, *ret_dict_dict2_userdef;
> - QInt *ret3;
> + QNum *ret3;
> + int64_t val;
>
> qdict_put_int(ud1a, "integer", 42);
> qdict_put_str(ud1a, "string", "hello");
> @@ -194,8 +195,9 @@ static void test_dispatch_cmd_io(void)
> qdict_put(req, "arguments", args3);
> qdict_put_str(req, "execute", "guest-get-time");
>
> - ret3 = qobject_to_qint(test_qmp_dispatch(req));
> - assert(qint_get_int(ret3) == 66);
> + ret3 = qobject_to_qnum(test_qmp_dispatch(req));
> + g_assert(qnum_get_int(ret3, &val));
> + g_assert_cmpint(val, ==, 66);
> QDECREF(ret3);
>
> QDECREF(req);
> diff --git a/tests/test-qmp-event.c b/tests/test-qmp-event.c
> index 4c0f09601d..c6a5ff38d0 100644
> --- a/tests/test-qmp-event.c
> +++ b/tests/test-qmp-event.c
> @@ -18,7 +18,6 @@
> #include "test-qapi-visit.h"
> #include "test-qapi-event.h"
> #include "qapi/qmp/types.h"
> -#include "qapi/qmp/qint.h"
> #include "qapi/qmp/qobject.h"
> #include "qapi/qmp-event.h"
>
> @@ -41,6 +40,7 @@ void qdict_cmp_do_simple(const char *key, QObject *obj1,
> void *opaque)
> {
> QObject *obj2;
> QDictCmpData d_new, *d = opaque;
> + int64_t val1, val2;
>
> if (!d->result) {
> return;
> @@ -62,9 +62,10 @@ void qdict_cmp_do_simple(const char *key, QObject *obj1,
> void *opaque)
> d->result = (qbool_get_bool(qobject_to_qbool(obj1)) ==
> qbool_get_bool(qobject_to_qbool(obj2)));
> return;
> - case QTYPE_QINT:
> - d->result = (qint_get_int(qobject_to_qint(obj1)) ==
> - qint_get_int(qobject_to_qint(obj2)));
> + case QTYPE_QNUM:
> + g_assert(qnum_get_int(qobject_to_qnum(obj1), &val1));
> + g_assert(qnum_get_int(qobject_to_qnum(obj2), &val2));
> + d->result = val1 == val2;
> return;
> case QTYPE_QSTRING:
> d->result = g_strcmp0(qstring_get_str(qobject_to_qstring(obj1)),
> diff --git a/tests/test-qobject-input-visitor.c
> b/tests/test-qobject-input-visitor.c
> index f2ed3161af..e6757727fd 100644
> --- a/tests/test-qobject-input-visitor.c
> +++ b/tests/test-qobject-input-visitor.c
> @@ -164,7 +164,7 @@ static void
> test_visitor_in_int_overflow(TestInputVisitorData *data,
> Visitor *v;
>
> /* this will overflow a Qint/int64, so should be deserialized into
s#a Qint/int64#an int64_t#
s#a QNum/double field#a double#
Or perhaps explain using QNUM_I64 and QNUM_DOUBLE. Your choice.
> - * a QFloat/double field instead, leading to an error if we pass it
> + * a QNum/double field instead, leading to an error if we pass it
> * to visit_type_int. confirm this.
> */
> v = visitor_input_test_init(data, "%f", DBL_MAX);
> @@ -466,17 +466,19 @@ static void test_visitor_in_any(TestInputVisitorData
> *data,
> {
> QObject *res = NULL;
> Visitor *v;
> - QInt *qint;
> + QNum *qnum;
> QBool *qbool;
> QString *qstring;
> QDict *qdict;
> QObject *qobj;
> + int64_t val;
>
> v = visitor_input_test_init(data, "-42");
> visit_type_any(v, NULL, &res, &error_abort);
> - qint = qobject_to_qint(res);
> - g_assert(qint);
> - g_assert_cmpint(qint_get_int(qint), ==, -42);
> + qnum = qobject_to_qnum(res);
> + g_assert(qnum);
> + g_assert(qnum_get_int(qnum, &val));
> + g_assert_cmpint(val, ==, -42);
> qobject_decref(res);
>
> v = visitor_input_test_init(data, "{ 'integer': -42, 'boolean': true,
> 'string': 'foo' }");
> @@ -485,9 +487,10 @@ static void test_visitor_in_any(TestInputVisitorData
> *data,
> g_assert(qdict && qdict_size(qdict) == 3);
> qobj = qdict_get(qdict, "integer");
> g_assert(qobj);
> - qint = qobject_to_qint(qobj);
> - g_assert(qint);
> - g_assert_cmpint(qint_get_int(qint), ==, -42);
> + qnum = qobject_to_qnum(qobj);
> + g_assert(qnum);
> + g_assert(qnum_get_int(qnum, &val));
> + g_assert_cmpint(val, ==, -42);
> qobj = qdict_get(qdict, "boolean");
> g_assert(qobj);
> qbool = qobject_to_qbool(qobj);
> @@ -565,7 +568,7 @@ static void
> test_visitor_in_alternate(TestInputVisitorData *data,
>
> v = visitor_input_test_init(data, "42");
> visit_type_UserDefAlternate(v, NULL, &tmp, &error_abort);
> - g_assert_cmpint(tmp->type, ==, QTYPE_QINT);
> + g_assert_cmpint(tmp->type, ==, QTYPE_QNUM);
> g_assert_cmpint(tmp->u.i, ==, 42);
> qapi_free_UserDefAlternate(tmp);
>
> @@ -593,7 +596,7 @@ static void
> test_visitor_in_alternate(TestInputVisitorData *data,
>
> v = visitor_input_test_init(data, "{ 'alt': 42 }");
> visit_type_WrapAlternate(v, NULL, &wrap, &error_abort);
> - g_assert_cmpint(wrap->alt->type, ==, QTYPE_QINT);
> + g_assert_cmpint(wrap->alt->type, ==, QTYPE_QNUM);
> g_assert_cmpint(wrap->alt->u.i, ==, 42);
> qapi_free_WrapAlternate(wrap);
>
> @@ -634,19 +637,19 @@ static void
> test_visitor_in_alternate_number(TestInputVisitorData *data,
>
> v = visitor_input_test_init(data, "42");
> visit_type_AltEnumNum(v, NULL, &aen, &error_abort);
> - g_assert_cmpint(aen->type, ==, QTYPE_QFLOAT);
> + g_assert_cmpint(aen->type, ==, QTYPE_QNUM);
> g_assert_cmpfloat(aen->u.n, ==, 42);
> qapi_free_AltEnumNum(aen);
>
> v = visitor_input_test_init(data, "42");
> visit_type_AltNumEnum(v, NULL, &ans, &error_abort);
> - g_assert_cmpint(ans->type, ==, QTYPE_QFLOAT);
> + g_assert_cmpint(ans->type, ==, QTYPE_QNUM);
> g_assert_cmpfloat(ans->u.n, ==, 42);
> qapi_free_AltNumEnum(ans);
>
> v = visitor_input_test_init(data, "42");
> visit_type_AltEnumInt(v, NULL, &asi, &error_abort);
> - g_assert_cmpint(asi->type, ==, QTYPE_QINT);
> + g_assert_cmpint(asi->type, ==, QTYPE_QNUM);
> g_assert_cmpint(asi->u.i, ==, 42);
> qapi_free_AltEnumInt(asi);
>
> @@ -659,13 +662,13 @@ static void
> test_visitor_in_alternate_number(TestInputVisitorData *data,
>
> v = visitor_input_test_init(data, "42.5");
> visit_type_AltEnumNum(v, NULL, &aen, &error_abort);
> - g_assert_cmpint(aen->type, ==, QTYPE_QFLOAT);
> + g_assert_cmpint(aen->type, ==, QTYPE_QNUM);
> g_assert_cmpfloat(aen->u.n, ==, 42.5);
> qapi_free_AltEnumNum(aen);
>
> v = visitor_input_test_init(data, "42.5");
> visit_type_AltNumEnum(v, NULL, &ans, &error_abort);
> - g_assert_cmpint(ans->type, ==, QTYPE_QFLOAT);
> + g_assert_cmpint(ans->type, ==, QTYPE_QNUM);
> g_assert_cmpfloat(ans->u.n, ==, 42.5);
> qapi_free_AltNumEnum(ans);
>
> diff --git a/tests/test-qobject-output-visitor.c
> b/tests/test-qobject-output-visitor.c
> index 4e8fdf1397..3180d8cbde 100644
> --- a/tests/test-qobject-output-visitor.c
> +++ b/tests/test-qobject-output-visitor.c
> @@ -58,13 +58,15 @@ static void test_visitor_out_int(TestOutputVisitorData
> *data,
> const void *unused)
> {
> int64_t value = -42;
> - QInt *qint;
> + int64_t val;
> + QNum *qnum;
>
> visit_type_int(data->ov, NULL, &value, &error_abort);
>
> - qint = qobject_to_qint(visitor_get(data));
> - g_assert(qint);
> - g_assert_cmpint(qint_get_int(qint), ==, value);
> + qnum = qobject_to_qnum(visitor_get(data));
> + g_assert(qnum);
> + g_assert(qnum_get_int(qnum, &val));
> + g_assert_cmpint(val, ==, value);
> }
>
> static void test_visitor_out_bool(TestOutputVisitorData *data,
> @@ -84,13 +86,13 @@ static void test_visitor_out_number(TestOutputVisitorData
> *data,
> const void *unused)
> {
> double value = 3.14;
> - QFloat *qfloat;
> + QNum *qnum;
>
> visit_type_number(data->ov, NULL, &value, &error_abort);
>
> - qfloat = qobject_to_qfloat(visitor_get(data));
> - g_assert(qfloat);
> - g_assert(qfloat_get_double(qfloat) == value);
> + qnum = qobject_to_qnum(visitor_get(data));
> + g_assert(qnum);
> + g_assert(qnum_get_double(qnum) == value);
> }
>
> static void test_visitor_out_string(TestOutputVisitorData *data,
> @@ -329,16 +331,18 @@ static void test_visitor_out_any(TestOutputVisitorData
> *data,
> const void *unused)
> {
> QObject *qobj;
> - QInt *qint;
> + QNum *qnum;
> QBool *qbool;
> QString *qstring;
> QDict *qdict;
> + int64_t val;
>
> - qobj = QOBJECT(qint_from_int(-42));
> + qobj = QOBJECT(qnum_from_int(-42));
> visit_type_any(data->ov, NULL, &qobj, &error_abort);
> - qint = qobject_to_qint(visitor_get(data));
> - g_assert(qint);
> - g_assert_cmpint(qint_get_int(qint), ==, -42);
> + qnum = qobject_to_qnum(visitor_get(data));
> + g_assert(qnum);
> + g_assert(qnum_get_int(qnum, &val));
> + g_assert_cmpint(val, ==, -42);
> qobject_decref(qobj);
>
> visitor_reset(data);
> @@ -351,9 +355,10 @@ static void test_visitor_out_any(TestOutputVisitorData
> *data,
> qobject_decref(qobj);
> qdict = qobject_to_qdict(visitor_get(data));
> g_assert(qdict);
> - qint = qobject_to_qint(qdict_get(qdict, "integer"));
> - g_assert(qint);
> - g_assert_cmpint(qint_get_int(qint), ==, -42);
> + qnum = qobject_to_qnum(qdict_get(qdict, "integer"));
> + g_assert(qnum);
> + g_assert(qnum_get_int(qnum, &val));
> + g_assert_cmpint(val, ==, -42);
> qbool = qobject_to_qbool(qdict_get(qdict, "boolean"));
> g_assert(qbool);
> g_assert(qbool_get_bool(qbool) == true);
> @@ -388,18 +393,20 @@ static void
> test_visitor_out_alternate(TestOutputVisitorData *data,
> const void *unused)
> {
> UserDefAlternate *tmp;
> - QInt *qint;
> + QNum *qnum;
> QString *qstr;
> QDict *qdict;
> + int64_t val;
>
> tmp = g_new0(UserDefAlternate, 1);
> - tmp->type = QTYPE_QINT;
> + tmp->type = QTYPE_QNUM;
> tmp->u.i = 42;
>
> visit_type_UserDefAlternate(data->ov, NULL, &tmp, &error_abort);
> - qint = qobject_to_qint(visitor_get(data));
> - g_assert(qint);
> - g_assert_cmpint(qint_get_int(qint), ==, 42);
> + qnum = qobject_to_qnum(visitor_get(data));
> + g_assert(qnum);
> + g_assert(qnum_get_int(qnum, &val));
> + g_assert_cmpint(val, ==, 42);
>
> qapi_free_UserDefAlternate(tmp);
>
> @@ -603,18 +610,21 @@ static void check_native_list(QObject *qobj,
> case USER_DEF_NATIVE_LIST_UNION_KIND_U16:
> case USER_DEF_NATIVE_LIST_UNION_KIND_U32:
> case USER_DEF_NATIVE_LIST_UNION_KIND_U64:
> - /* all integer elements in JSON arrays get stored into QInts when
> + /* all integer elements in JSON arrays get stored into QNums when
Please use the opportunity to wing the comment at both ends and start
with a capital letter:
/*
* All integer ...
> * we convert to QObjects, so we can check them all in the same
> * fashion, so simply fall through here
> */
> case USER_DEF_NATIVE_LIST_UNION_KIND_INTEGER:
> for (i = 0; i < 32; i++) {
> QObject *tmp;
> - QInt *qvalue;
> + QNum *qvalue;
> + int64_t val;
> +
> tmp = qlist_peek(qlist);
> g_assert(tmp);
> - qvalue = qobject_to_qint(tmp);
> - g_assert_cmpint(qint_get_int(qvalue), ==, i);
> + qvalue = qobject_to_qnum(tmp);
> + g_assert(qnum_get_int(qvalue, &val));
> + g_assert_cmpint(val, ==, i);
> qobject_decref(qlist_pop(qlist));
> }
> break;
> @@ -645,15 +655,15 @@ static void check_native_list(QObject *qobj,
> case USER_DEF_NATIVE_LIST_UNION_KIND_NUMBER:
> for (i = 0; i < 32; i++) {
> QObject *tmp;
> - QFloat *qvalue;
> + QNum *qvalue;
> GString *double_expected = g_string_new("");
> GString *double_actual = g_string_new("");
>
> tmp = qlist_peek(qlist);
> g_assert(tmp);
> - qvalue = qobject_to_qfloat(tmp);
> + qvalue = qobject_to_qnum(tmp);
> g_string_printf(double_expected, "%.6f", (double)i / 3);
> - g_string_printf(double_actual, "%.6f",
> qfloat_get_double(qvalue));
> + g_string_printf(double_actual, "%.6f", qnum_get_double(qvalue));
> g_assert_cmpstr(double_actual->str, ==, double_expected->str);
>
> qobject_decref(qlist_pop(qlist));
> diff --git a/tests/test-x86-cpuid-compat.c b/tests/test-x86-cpuid-compat.c
> index 6c71e46391..f30f2030a8 100644
> --- a/tests/test-x86-cpuid-compat.c
> +++ b/tests/test-x86-cpuid-compat.c
> @@ -1,10 +1,7 @@
> #include "qemu/osdep.h"
> #include "qemu-common.h"
> -#include "qapi/qmp/qlist.h"
> -#include "qapi/qmp/qstring.h"
> -#include "qapi/qmp/qdict.h"
> -#include "qapi/qmp/qint.h"
> -#include "qapi/qmp/qbool.h"
> +#include "qapi/error.h"
> +#include "qapi/qmp/types.h"
Previous note on qapi/qmp/types.h applies.
> #include "libqtest.h"
>
> static char *get_cpu0_qom_path(void)
> @@ -57,12 +54,14 @@ static void test_cpuid_prop(const void *data)
> {
> const CpuidTestArgs *args = data;
> char *path;
> - QInt *value;
> + QNum *value;
> + int64_t val;
>
> qtest_start(args->cmdline);
> path = get_cpu0_qom_path();
> - value = qobject_to_qint(qom_get(path, args->property));
> - g_assert_cmpint(qint_get_int(value), ==, args->expected_value);
> + value = qobject_to_qnum(qom_get(path, args->property));
> + g_assert(qnum_get_int(value, &val));
> + g_assert_cmpint(val, ==, args->expected_value);
> qtest_end();
>
> QDECREF(value);
> @@ -109,12 +108,15 @@ static uint32_t get_feature_word(QList *features,
> uint32_t eax, uint32_t ecx,
> uint32_t reax = qdict_get_int(w, "cpuid-input-eax");
> bool has_ecx = qdict_haskey(w, "cpuid-input-ecx");
> uint32_t recx = 0;
> + int64_t val;
>
> if (has_ecx) {
> recx = qdict_get_int(w, "cpuid-input-ecx");
> }
> if (eax == reax && (!has_ecx || ecx == recx) && !strcmp(rreg, reg)) {
> - return qint_get_int(qobject_to_qint(qdict_get(w, "features")));
> + g_assert(qnum_get_int(qobject_to_qnum(qdict_get(w, "features")),
> + &val));
> + return val;
> }
> }
> return 0;
> diff --git a/ui/spice-core.c b/ui/spice-core.c
> index 804abc5c0f..561d0649cf 100644
> --- a/ui/spice-core.c
> +++ b/ui/spice-core.c
> @@ -30,7 +30,6 @@
> #include "qemu-x509.h"
> #include "qemu/sockets.h"
> #include "qmp-commands.h"
> -#include "qapi/qmp/qint.h"
> #include "qapi/qmp/qbool.h"
> #include "qapi/qmp/qstring.h"
> #include "qapi/qmp/qjson.h"
> diff --git a/ui/vnc-enc-tight.c b/ui/vnc-enc-tight.c
> index 1e53b1cf84..89ab12c0d8 100644
> --- a/ui/vnc-enc-tight.c
> +++ b/ui/vnc-enc-tight.c
> @@ -44,7 +44,6 @@
> #endif
>
> #include "qemu/bswap.h"
> -#include "qapi/qmp/qint.h"
> #include "vnc.h"
> #include "vnc-enc-tight.h"
> #include "vnc-palette.h"
> diff --git a/util/qemu-option.c b/util/qemu-option.c
> index 5977bfc3e9..39b1e06225 100644
> --- a/util/qemu-option.c
> +++ b/util/qemu-option.c
> @@ -941,9 +941,8 @@ typedef struct OptsFromQDictState {
> static void qemu_opts_from_qdict_1(const char *key, QObject *obj, void
> *opaque)
> {
> OptsFromQDictState *state = opaque;
> - char buf[32];
> + char buf[32], *tmp = NULL;
> const char *value;
> - int n;
>
> if (!strcmp(key, "id") || *state->errp) {
> return;
> @@ -953,17 +952,9 @@ static void qemu_opts_from_qdict_1(const char *key,
> QObject *obj, void *opaque)
> case QTYPE_QSTRING:
> value = qstring_get_str(qobject_to_qstring(obj));
> break;
> - case QTYPE_QINT:
> - n = snprintf(buf, sizeof(buf), "%" PRId64,
> - qint_get_int(qobject_to_qint(obj)));
> - assert(n < sizeof(buf));
> - value = buf;
> - break;
> - case QTYPE_QFLOAT:
> - n = snprintf(buf, sizeof(buf), "%.17g",
> - qfloat_get_double(qobject_to_qfloat(obj)));
> - assert(n < sizeof(buf));
> - value = buf;
> + case QTYPE_QNUM:
> + tmp = qnum_to_string(qobject_to_qnum(obj));
> + value = tmp;
> break;
> case QTYPE_QBOOL:
> pstrcpy(buf, sizeof(buf),
> @@ -975,12 +966,13 @@ static void qemu_opts_from_qdict_1(const char *key,
> QObject *obj, void *opaque)
> }
>
> qemu_opt_set(state->opts, key, value, state->errp);
> + g_free(tmp);
> }
>
> /*
> * Create QemuOpts from a QDict.
> * Use value of key "id" as ID if it exists and is a QString.
> - * Only QStrings, QInts, QFloats and QBools are copied. Entries with
> + * Only QStrings, QNums and QBools are copied. Entries with
> * other types are silently ignored.
> */
Refill the sentence, please.
> QemuOpts *qemu_opts_from_qdict(QemuOptsList *list, const QDict *qdict,
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 7df088259b..b53054111a 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -1412,8 +1412,7 @@ F: include/qapi/qmp/
> X: include/qapi/qmp/dispatch.h
> F: scripts/coccinelle/qobject.cocci
> F: tests/check-qdict.c
> -F: tests/check-qfloat.c
> -F: tests/check-qint.c
> +F: tests/check-qnum.c
> F: tests/check-qjson.c
> F: tests/check-qlist.c
> F: tests/check-qstring.c
> diff --git a/qobject/Makefile.objs b/qobject/Makefile.objs
> index bed55084bb..fc8885c9a4 100644
> --- a/qobject/Makefile.objs
> +++ b/qobject/Makefile.objs
> @@ -1,2 +1,2 @@
> -util-obj-y = qnull.o qint.o qstring.o qdict.o qlist.o qfloat.o qbool.o
> +util-obj-y = qnull.o qnum.o qstring.o qdict.o qlist.o qbool.o
> util-obj-y += qjson.o qobject.o json-lexer.o json-streamer.o json-parser.o
> diff --git a/scripts/coccinelle/qobject.cocci
> b/scripts/coccinelle/qobject.cocci
> index 97703a438b..c3253deb1b 100644
> --- a/scripts/coccinelle/qobject.cocci
> +++ b/scripts/coccinelle/qobject.cocci
> @@ -6,7 +6,7 @@ expression Obj, Key, E;
> - qdict_put_obj(Obj, Key, QOBJECT(E));
> + qdict_put(Obj, Key, E);
> |
> -- qdict_put(Obj, Key, qint_from_int(E));
> +- qdict_put(Obj, Key, qnum_from_int(E));
> + qdict_put_int(Obj, Key, E);
> |
> - qdict_put(Obj, Key, qbool_from_bool(E));
> @@ -24,7 +24,7 @@ expression Obj, E;
> - qlist_append_obj(Obj, QOBJECT(E));
> + qlist_append(Obj, E);
> |
> -- qlist_append(Obj, qint_from_int(E));
> +- qlist_append(Obj, qnum_from_int(E));
> + qlist_append_int(Obj, E);
> |
> - qlist_append(Obj, qbool_from_bool(E));
> diff --git a/tests/.gitignore b/tests/.gitignore
> index 40c2e3e757..8e01b004f1 100644
> --- a/tests/.gitignore
> +++ b/tests/.gitignore
> @@ -1,7 +1,6 @@
> atomic_add-bench
> check-qdict
> -check-qfloat
> -check-qint
> +check-qnum
> check-qjson
> check-qlist
> check-qnull
> diff --git a/tests/Makefile.include b/tests/Makefile.include
> index f42f3dfa72..fec5af765a 100644
> --- a/tests/Makefile.include
> +++ b/tests/Makefile.include
> @@ -10,10 +10,8 @@ check-unit-y = tests/check-qdict$(EXESUF)
> gcov-files-check-qdict-y = qobject/qdict.c
> check-unit-y += tests/test-char$(EXESUF)
> gcov-files-check-qdict-y = chardev/char.c
> -check-unit-y += tests/check-qfloat$(EXESUF)
> -gcov-files-check-qfloat-y = qobject/qfloat.c
> -check-unit-y += tests/check-qint$(EXESUF)
> -gcov-files-check-qint-y = qobject/qint.c
> +check-unit-y += tests/check-qnum$(EXESUF)
> +gcov-files-check-qnum-y = qobject/qnum.c
> check-unit-y += tests/check-qstring$(EXESUF)
> gcov-files-check-qstring-y = qobject/qstring.c
> check-unit-y += tests/check-qlist$(EXESUF)
> @@ -506,8 +504,8 @@ GENERATED_FILES += tests/test-qapi-types.h
> tests/test-qapi-visit.h \
> tests/test-qmp-commands.h tests/test-qapi-event.h \
> tests/test-qmp-introspect.h
>
> -test-obj-y = tests/check-qint.o tests/check-qstring.o tests/check-qdict.o \
> - tests/check-qlist.o tests/check-qfloat.o tests/check-qnull.o \
> +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/test-coroutine.o tests/test-string-output-visitor.o \
> tests/test-string-input-visitor.o tests/test-qobject-output-visitor.o \
> @@ -535,11 +533,10 @@ test-crypto-obj-y = $(crypto-obj-y) $(test-qom-obj-y)
> test-io-obj-y = $(io-obj-y) $(test-crypto-obj-y)
> test-block-obj-y = $(block-obj-y) $(test-io-obj-y) tests/iothread.o
>
> -tests/check-qint$(EXESUF): tests/check-qint.o $(test-util-obj-y)
> +tests/check-qnum$(EXESUF): tests/check-qnum.o $(test-util-obj-y)
> tests/check-qstring$(EXESUF): tests/check-qstring.o $(test-util-obj-y)
> 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-qfloat$(EXESUF): tests/check-qfloat.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-qom-interface$(EXESUF): tests/check-qom-interface.o
> $(test-qom-obj-y)
> diff --git a/tests/qapi-schema/comments.out b/tests/qapi-schema/comments.out
> index 5d7c13cad1..17e652535c 100644
> --- a/tests/qapi-schema/comments.out
> +++ b/tests/qapi-schema/comments.out
> @@ -1,4 +1,4 @@
> -enum QType ['none', 'qnull', 'qint', 'qstring', 'qdict', 'qlist', 'qfloat',
> 'qbool']
> +enum QType ['none', 'qnull', 'qnum', 'qstring', 'qdict', 'qlist', 'qbool']
> prefix QTYPE
> enum Status ['good', 'bad', 'ugly']
> object q_empty
> diff --git a/tests/qapi-schema/doc-good.out b/tests/qapi-schema/doc-good.out
> index 70c1252408..63ca25a8b9 100644
> --- a/tests/qapi-schema/doc-good.out
> +++ b/tests/qapi-schema/doc-good.out
> @@ -6,7 +6,7 @@ object Object
> tag base1
> case one: Variant1
> case two: Variant2
> -enum QType ['none', 'qnull', 'qint', 'qstring', 'qdict', 'qlist', 'qfloat',
> 'qbool']
> +enum QType ['none', 'qnull', 'qnum', 'qstring', 'qdict', 'qlist', 'qbool']
> prefix QTYPE
> object SugaredUnion
> member type: SugaredUnionKind optional=False
> diff --git a/tests/qapi-schema/empty.out b/tests/qapi-schema/empty.out
> index 8a5b034424..40b886ddae 100644
> --- a/tests/qapi-schema/empty.out
> +++ b/tests/qapi-schema/empty.out
> @@ -1,3 +1,3 @@
> -enum QType ['none', 'qnull', 'qint', 'qstring', 'qdict', 'qlist', 'qfloat',
> 'qbool']
> +enum QType ['none', 'qnull', 'qnum', 'qstring', 'qdict', 'qlist', 'qbool']
> prefix QTYPE
> object q_empty
> diff --git a/tests/qapi-schema/event-case.out
> b/tests/qapi-schema/event-case.out
> index 5a0f2bf805..313c0fe7be 100644
> --- a/tests/qapi-schema/event-case.out
> +++ b/tests/qapi-schema/event-case.out
> @@ -1,4 +1,4 @@
> -enum QType ['none', 'qnull', 'qint', 'qstring', 'qdict', 'qlist', 'qfloat',
> 'qbool']
> +enum QType ['none', 'qnull', 'qnum', 'qstring', 'qdict', 'qlist', 'qbool']
> prefix QTYPE
> event oops None
> boxed=False
> diff --git a/tests/qapi-schema/ident-with-escape.out
> b/tests/qapi-schema/ident-with-escape.out
> index 1d2722c02e..b5637cb2e0 100644
> --- a/tests/qapi-schema/ident-with-escape.out
> +++ b/tests/qapi-schema/ident-with-escape.out
> @@ -1,4 +1,4 @@
> -enum QType ['none', 'qnull', 'qint', 'qstring', 'qdict', 'qlist', 'qfloat',
> 'qbool']
> +enum QType ['none', 'qnull', 'qnum', 'qstring', 'qdict', 'qlist', 'qbool']
> prefix QTYPE
> command fooA q_obj_fooA-arg -> None
> gen=True success_response=True boxed=False
> diff --git a/tests/qapi-schema/include-relpath.out
> b/tests/qapi-schema/include-relpath.out
> index 5d7c13cad1..17e652535c 100644
> --- a/tests/qapi-schema/include-relpath.out
> +++ b/tests/qapi-schema/include-relpath.out
> @@ -1,4 +1,4 @@
> -enum QType ['none', 'qnull', 'qint', 'qstring', 'qdict', 'qlist', 'qfloat',
> 'qbool']
> +enum QType ['none', 'qnull', 'qnum', 'qstring', 'qdict', 'qlist', 'qbool']
> prefix QTYPE
> enum Status ['good', 'bad', 'ugly']
> object q_empty
> diff --git a/tests/qapi-schema/include-repetition.out
> b/tests/qapi-schema/include-repetition.out
> index 5d7c13cad1..17e652535c 100644
> --- a/tests/qapi-schema/include-repetition.out
> +++ b/tests/qapi-schema/include-repetition.out
> @@ -1,4 +1,4 @@
> -enum QType ['none', 'qnull', 'qint', 'qstring', 'qdict', 'qlist', 'qfloat',
> 'qbool']
> +enum QType ['none', 'qnull', 'qnum', 'qstring', 'qdict', 'qlist', 'qbool']
> prefix QTYPE
> enum Status ['good', 'bad', 'ugly']
> object q_empty
> diff --git a/tests/qapi-schema/include-simple.out
> b/tests/qapi-schema/include-simple.out
> index 5d7c13cad1..17e652535c 100644
> --- a/tests/qapi-schema/include-simple.out
> +++ b/tests/qapi-schema/include-simple.out
> @@ -1,4 +1,4 @@
> -enum QType ['none', 'qnull', 'qint', 'qstring', 'qdict', 'qlist', 'qfloat',
> 'qbool']
> +enum QType ['none', 'qnull', 'qnum', 'qstring', 'qdict', 'qlist', 'qbool']
> prefix QTYPE
> enum Status ['good', 'bad', 'ugly']
> object q_empty
> diff --git a/tests/qapi-schema/indented-expr.out
> b/tests/qapi-schema/indented-expr.out
> index e8171c935f..586795f44d 100644
> --- a/tests/qapi-schema/indented-expr.out
> +++ b/tests/qapi-schema/indented-expr.out
> @@ -1,4 +1,4 @@
> -enum QType ['none', 'qnull', 'qint', 'qstring', 'qdict', 'qlist', 'qfloat',
> 'qbool']
> +enum QType ['none', 'qnull', 'qnum', 'qstring', 'qdict', 'qlist', 'qbool']
> prefix QTYPE
> command eins None -> None
> gen=True success_response=True boxed=False
> diff --git a/tests/qapi-schema/qapi-schema-test.out
> b/tests/qapi-schema/qapi-schema-test.out
> index e727a5a84c..b88b8aae6f 100644
> --- a/tests/qapi-schema/qapi-schema-test.out
> +++ b/tests/qapi-schema/qapi-schema-test.out
> @@ -50,7 +50,7 @@ object NestedEnumsOne
> member enum4: EnumOne optional=True
> enum QEnumTwo ['value1', 'value2']
> prefix QENUM_TWO
> -enum QType ['none', 'qnull', 'qint', 'qstring', 'qdict', 'qlist', 'qfloat',
> 'qbool']
> +enum QType ['none', 'qnull', 'qnum', 'qstring', 'qdict', 'qlist', 'qbool']
> prefix QTYPE
> object TestStruct
> member integer: int optional=False
| [Prev in Thread] |
Current Thread |
[Next in Thread] |
- Re: [Qemu-devel] [PATCH v2 09/45] qapi: merge QInt and QFloat in QNum,
Markus Armbruster <=