| Message ID | 20170311132256.22951-5-marcandre.lureau@redhat.com |
|---|---|
| State | New |
| Headers | show |
On 03/11/2017 07:22 AM, Marc-André Lureau wrote:
> The type is not used at all yet. Add some tests to exercice it.
s/exercice/exercise/
I wonder if we need this patch at all.
I've been thinking about a possible alternative representation, such
that a single QInt type can cover _both_ signed and unsigned types, by
having QInt track a sign bit bool in addition to a uint64_t value.
If the user passes in a value in the range [0, INT64_MAX], the number is
representable in both uint64_t and int64_t. So the internal sign bit
would be omitted, as in { value=v, sign=false }. Input visitors can
visit this value with both the 'uint64' and the 'int64' visitors, and
output visitors will output the same value.
If the user passes in a value in the range [-INT64_MAX-1, -1], the
number is intended to be used as a negative signed number. So the QInt
representation would be the 2s complement representation, { value=-v,
sign=true }. On input, the uint64_t visitor would reject the value as
out of range, the int64_t visitor would return the appropriate negative
value, and the generic int visitor will return the uint64_t value. On
output, the number would be given the negative signed value
representation in anything that stringizes the value.
If the user passes in a value in the range [INT64_MAX+1, UINT64_MAX},
the number is intended to be used as a large unsigned number. So the
QInt representation would be { value=v, sign=false }. On input, the
uint64_t visitor would accept the value, the int64_t visitor would
reject it as out of range, and the generic int visitor will return the
uint64_t value. On output, the number would be given the positive
unsigned value representation in the stringized form.
That way, you don't have to complicate the (already-awkward) type
promotion rules of QFloat vs QInt with yet another scalar type. The
input parser becomes friendlier - you can either use generic 'int' when
you want to allow the user to pass in either signed or unsigned values
(important for back-compat, where callers like libvirt learned that only
signed values worked even when large unsigned values were wanted); or
the specific 'int64' and 'uint64' types to force proper ranges.
It's definitely an alternative implementation to the one you pursued in
your series, so maybe we should first figure out WHAT we want
represented, before implementing something that will be more baggage
than we want (you at least have the advantage of patches that are
written; my idea of adding a sign bool to QInt is still just in the
initial idea stage).
Eric Blake <eblake@redhat.com> writes: > On 03/11/2017 07:22 AM, Marc-André Lureau wrote: >> The type is not used at all yet. Add some tests to exercice it. > > s/exercice/exercise/ > > I wonder if we need this patch at all. > > I've been thinking about a possible alternative representation, such > that a single QInt type can cover _both_ signed and unsigned types, by > having QInt track a sign bit bool in addition to a uint64_t value. > > If the user passes in a value in the range [0, INT64_MAX], the number is > representable in both uint64_t and int64_t. So the internal sign bit > would be omitted, as in { value=v, sign=false }. Input visitors can > visit this value with both the 'uint64' and the 'int64' visitors, and > output visitors will output the same value. > > If the user passes in a value in the range [-INT64_MAX-1, -1], the > number is intended to be used as a negative signed number. So the QInt > representation would be the 2s complement representation, { value=-v, > sign=true }. On input, the uint64_t visitor would reject the value as > out of range, the int64_t visitor would return the appropriate negative > value, and the generic int visitor will return the uint64_t value. On > output, the number would be given the negative signed value > representation in anything that stringizes the value. > > If the user passes in a value in the range [INT64_MAX+1, UINT64_MAX}, > the number is intended to be used as a large unsigned number. So the > QInt representation would be { value=v, sign=false }. On input, the > uint64_t visitor would accept the value, the int64_t visitor would > reject it as out of range, and the generic int visitor will return the > uint64_t value. On output, the number would be given the positive > unsigned value representation in the stringized form. > > That way, you don't have to complicate the (already-awkward) type > promotion rules of QFloat vs QInt with yet another scalar type. The > input parser becomes friendlier - you can either use generic 'int' when > you want to allow the user to pass in either signed or unsigned values > (important for back-compat, where callers like libvirt learned that only > signed values worked even when large unsigned values were wanted); or > the specific 'int64' and 'uint64' types to force proper ranges. > > It's definitely an alternative implementation to the one you pursued in > your series, so maybe we should first figure out WHAT we want > represented, before implementing something that will be more baggage > than we want (you at least have the advantage of patches that are > written; my idea of adding a sign bool to QInt is still just in the > initial idea stage). RFC 7159 gives implementations wide latitude on number representation (this is one of the several reasons why JSON is a poor standard from an interoperability point of view). Our current representation effectively is "if it fits into int64_t, use that, else double". What we need is something that can represent uint64_t exactly, too. You say you've been thinking about extending QInt's range to cover uint64_t. I've been thinking even more radically: replace both QInt and QFloat by QNumber. This is how JSON *actually* works. The new QNumber type provides constructors from double, int64_t and uint64_t. It also provides conversion functions to double, int64_t and uint64_t. The latter two can fail. I'd expect these to be perfectly servicable for the visitors. Code messing with QObject directly might become slightly more elaborate. But such code should be minimized anyway.
On 03/13/2017 02:15 AM, Markus Armbruster wrote: > Eric Blake <eblake@redhat.com> writes: > >> On 03/11/2017 07:22 AM, Marc-André Lureau wrote: >>> The type is not used at all yet. Add some tests to exercice it. >> >> s/exercice/exercise/ >> >> I wonder if we need this patch at all. >> >> I've been thinking about a possible alternative representation, such >> that a single QInt type can cover _both_ signed and unsigned types, by >> having QInt track a sign bit bool in addition to a uint64_t value. >> > You say you've been thinking about extending QInt's range to cover > uint64_t. I've been thinking even more radically: replace both QInt and > QFloat by QNumber. This is how JSON *actually* works. > > The new QNumber type provides constructors from double, int64_t and > uint64_t. It also provides conversion functions to double, int64_t and > uint64_t. The latter two can fail. Interesting - I like it, as it takes my idea and goes one step further. You'd want to track 64 bits of precision rather than just 53, when the input was integral, but the idea seems to have some merit (we have some special case in the testsuite for what happens in alternates with various combinations of 'number' vs. 'int' that may need tweaking when they are no longer distinguishable as QInt vs QFloat, but that's not too onerous).
Hi ----- Original Message ----- > On 03/13/2017 02:15 AM, Markus Armbruster wrote: > > Eric Blake <eblake@redhat.com> writes: > > > >> On 03/11/2017 07:22 AM, Marc-André Lureau wrote: > >>> The type is not used at all yet. Add some tests to exercice it. > >> > >> s/exercice/exercise/ > >> > >> I wonder if we need this patch at all. > >> > >> I've been thinking about a possible alternative representation, such > >> that a single QInt type can cover _both_ signed and unsigned types, by > >> having QInt track a sign bit bool in addition to a uint64_t value. > >> > > > You say you've been thinking about extending QInt's range to cover > > uint64_t. I've been thinking even more radically: replace both QInt and > > QFloat by QNumber. This is how JSON *actually* works. > > > > The new QNumber type provides constructors from double, int64_t and > > uint64_t. It also provides conversion functions to double, int64_t and > > uint64_t. The latter two can fail. > > Interesting - I like it, as it takes my idea and goes one step further. > You'd want to track 64 bits of precision rather than just 53, when the > input was integral, but the idea seems to have some merit (we have some > special case in the testsuite for what happens in alternates with > various combinations of 'number' vs. 'int' that may need tweaking when > they are no longer distinguishable as QInt vs QFloat, but that's not too > onerous). > I wonder the benefits from hiding the real type behind a QNumber "superclass", then having to type check at a lower level. QType is not only used for json, so I see some benefits from having a bit stricter type declaration and compile-time check. But I don't have a very good idea of what it would mean to have a generic QNumber type, I could try to implement it to have a more informed opinion. > -- > Eric Blake eblake redhat com +1-919-301-3266 > Libvirt virtualization library http://libvirt.org > >
Hi On Mon, Mar 13, 2017 at 5:29 PM Marc-André Lureau <mlureau@redhat.com> wrote: Hi ----- Original Message ----- > On 03/13/2017 02:15 AM, Markus Armbruster wrote: > > Eric Blake <eblake@redhat.com> writes: > > > >> On 03/11/2017 07:22 AM, Marc-André Lureau wrote: > >>> The type is not used at all yet. Add some tests to exercice it. > >> > >> s/exercice/exercise/ > >> > >> I wonder if we need this patch at all. > >> > >> I've been thinking about a possible alternative representation, such > >> that a single QInt type can cover _both_ signed and unsigned types, by > >> having QInt track a sign bit bool in addition to a uint64_t value. > >> > > > You say you've been thinking about extending QInt's range to cover > > uint64_t. I've been thinking even more radically: replace both QInt and > > QFloat by QNumber. This is how JSON *actually* works. > > > > The new QNumber type provides constructors from double, int64_t and > > uint64_t. It also provides conversion functions to double, int64_t and > > uint64_t. The latter two can fail. > > Interesting - I like it, as it takes my idea and goes one step further. > You'd want to track 64 bits of precision rather than just 53, when the > input was integral, but the idea seems to have some merit (we have some > special case in the testsuite for what happens in alternates with > various combinations of 'number' vs. 'int' that may need tweaking when > they are no longer distinguishable as QInt vs QFloat, but that's not too > onerous). > I wonder the benefits from hiding the real type behind a QNumber "superclass", then having to type check at a lower level. QType is not only used for json, so I see some benefits from having a bit stricter type declaration and compile-time check. But I don't have a very good idea of what it would mean to have a generic QNumber type, I could try to implement it to have a more informed opinion. I have looked a bit more into implementing a QNumber type. There is a bit more error handling to add everywhere for getting a value (for get_int, get_uint), as some conversions will fail. The qdict getters will have to throw those errors too. Whereas the QObject type check or dispatch is there, so less error handling to add. In my proposal, conversion from int to uint is done for positive values, with the qdict helper (is it the only way we access json parsed values?). Do we want to cast a negative int to a uint? Do we have qmp clients using this representation today, and can we break this? The qdev-properties will be slightly less convenient to define, but that's minor since it's behind macros, ex: - .qtype = QTYPE_QUINT, \ - .defval.u = (_type)_defval, \ + .qtype = QTYPE_QNUM, \ + .defval.type = QNUM_U64, \ + .defval.u.u64 = (_type)_defval, \ The biggest issue I have is how to handle the alt visitors if all ints are numbers, ex with AltIntNum: switch ((*obj)->type) { case QTYPE_QNUM: visit_type_int(v, name, &(*obj)->u.i, &err); break; case QTYPE_QNUM: visit_type_number(v, name, &(*obj)->u.n, &err); break; Should the generator have special handling for QNUM and dispatch based on underlying type? Overall, it seems to me that using QUINT like I proposed is more straightforward and equally convenient provided we have conversion helpers where needed (mostly for qdict). thanks for your comments
Marc-André Lureau <marcandre.lureau@gmail.com> writes: > Hi > > On Mon, Mar 13, 2017 at 5:29 PM Marc-André Lureau <mlureau@redhat.com> > wrote: > > Hi > > ----- Original Message ----- >> On 03/13/2017 02:15 AM, Markus Armbruster wrote: >> > Eric Blake <eblake@redhat.com> writes: >> > >> >> On 03/11/2017 07:22 AM, Marc-André Lureau wrote: >> >>> The type is not used at all yet. Add some tests to exercice it. >> >> >> >> s/exercice/exercise/ >> >> >> >> I wonder if we need this patch at all. >> >> >> >> I've been thinking about a possible alternative representation, such >> >> that a single QInt type can cover _both_ signed and unsigned types, by >> >> having QInt track a sign bit bool in addition to a uint64_t value. >> >> >> >> > You say you've been thinking about extending QInt's range to cover >> > uint64_t. I've been thinking even more radically: replace both QInt and >> > QFloat by QNumber. This is how JSON *actually* works. >> > >> > The new QNumber type provides constructors from double, int64_t and >> > uint64_t. It also provides conversion functions to double, int64_t and >> > uint64_t. The latter two can fail. >> >> Interesting - I like it, as it takes my idea and goes one step further. >> You'd want to track 64 bits of precision rather than just 53, when the >> input was integral, but the idea seems to have some merit (we have some >> special case in the testsuite for what happens in alternates with >> various combinations of 'number' vs. 'int' that may need tweaking when >> they are no longer distinguishable as QInt vs QFloat, but that's not too >> onerous). >> > > I wonder the benefits from hiding the real type behind a QNumber > "superclass", then having to type check at a lower level. QType is not only > used for json, The less it is used for non-JSON purposes, the better. > so I see some benefits from having a bit stricter type > declaration and compile-time check. But I don't have a very good idea of > what it would mean to have a generic QNumber type, I could try to implement > it to have a more informed opinion. > > > I have looked a bit more into implementing a QNumber type. > > There is a bit more error handling to add everywhere for getting a value > (for get_int, get_uint), as some conversions will fail. The qdict getters > will have to throw those errors too. Whereas the QObject type check or > dispatch is there, so less error handling to add. How many places convert QNumber to C integer type? How many of them have to do range checking anyway? If range checking turns out to be common, we could fuse it into the conversion, say return -EINVAL for qtype other than QNumber, and -ERANGE for a QNumber out of range, like qemu_strtol() does. > In my proposal, conversion from int to uint is done for positive values, > with the qdict helper (is it the only way we access json parsed values?). I doubt it. > Do we want to cast a negative int to a uint? Do we have qmp clients using > this representation today, and can we break this? We don't have "intepret negative as large unsigned" issues in the JSON parser, but we have them in the QObject input visitor: visit_type_uint64() rejects integers above INT64_MAX, but happily accepts integers between INT64_MIN and -1, and cast them to uint64_t. This makes no sense for QMP. It crept in right at the beginning because visitors were added without adequate test coverage. Eric noticed this problem and added the FIXME in commit f755dea, four and a half years later. We still lack an adequate test case. I'll add one. We're awfully slow learners. QMP clients that work around the "large positive integers are rejected" bugs by sending large negative ones instead may well exist. Fixing the interface would break them. Depressing. Eric, could you have a peek at libvirt? Additionally, direct users of QObject could conceivably convert QInt to int64_t, then silently cast to unsigned. Or narrower integer types; same bad idea, really. The only way to find them is to review all the code getting integers out of QObject. > The qdev-properties will be slightly less convenient to define, but that's > minor since it's behind macros, ex: > - .qtype = QTYPE_QUINT, \ > - .defval.u = (_type)_defval, \ > + .qtype = QTYPE_QNUM, \ > + .defval.type = QNUM_U64, \ > + .defval.u.u64 = (_type)_defval, \ > > The biggest issue I have is how to handle the alt visitors if all ints are > numbers, ex with AltIntNum: > > switch ((*obj)->type) { > case QTYPE_QNUM: > visit_type_int(v, name, &(*obj)->u.i, &err); > break; > case QTYPE_QNUM: > visit_type_number(v, name, &(*obj)->u.n, &err); > break; > > Should the generator have special handling for QNUM and dispatch based on > underlying type? I'm afraid we'll need that for keyval anyway. I've been playing with alternates a bit. I think visit_start_alternate() needs to be redone. Need to find time to finish my experiments and post patches. > Overall, it seems to me that using QUINT like I proposed is more > straightforward and equally convenient provided we have conversion helpers > where needed (mostly for qdict). Contradicts my gut feeling, but my gut is anything but infallible. I guess I'd like to see (sketches of) both, so we can make an informed decision. > thanks for your comments
On 03/21/2017 11:49 AM, Markus Armbruster wrote: > > QMP clients that work around the "large positive integers are rejected" > bugs by sending large negative ones instead may well exist. Fixing the > interface would break them. Depressing. Eric, could you have a peek at > libvirt? Yes, libvirt is such a client already. From util/virjson.c:virJSONValueObjectAddVArgs(): * Adds the key-value pairs supplied as variable argument list to @obj. * * Keys look like s:name the first letter is a type code: ... * U: unsigned long integer value (see below for quirks) * P: unsigned long integer value, omitted if zero ... case 'P': case 'U': { /* qemu silently truncates numbers larger than LLONG_MAX, * so passing the full range of unsigned 64 bit integers * is not safe here. Pass them as signed 64 bit integers * instead. */ long long val = va_arg(args, long long); if (!val && type == 'P') continue; rc = virJSONValueObjectAppendNumberLong(obj, key, val); } break; So if we "fix" QMP to reject negative values in place of large unsigned values, we'll need to also fix libvirt to have a way to introspect the difference and cope accordingly. (I'm thinking that we'll have to keep the negative parsing indefinitely, and can merely improve the parser to also parse large positive - but that we can still reject values < INT64_MIN that would have a weird wraparound).
Hi On Tue, Mar 21, 2017 at 9:06 PM Eric Blake <eblake@redhat.com> wrote: > On 03/21/2017 11:49 AM, Markus Armbruster wrote: > > > > > QMP clients that work around the "large positive integers are rejected" > > bugs by sending large negative ones instead may well exist. Fixing the > > interface would break them. Depressing. Eric, could you have a peek at > > libvirt? > > Yes, libvirt is such a client already. From > util/virjson.c:virJSONValueObjectAddVArgs(): > > > * Adds the key-value pairs supplied as variable argument list to @obj. > * > * Keys look like s:name the first letter is a type code: > ... > * U: unsigned long integer value (see below for quirks) > * P: unsigned long integer value, omitted if zero > ... > case 'P': > case 'U': { > /* qemu silently truncates numbers larger than LLONG_MAX, > * so passing the full range of unsigned 64 bit integers > * is not safe here. Pass them as signed 64 bit integers > * instead. > */ > long long val = va_arg(args, long long); > > if (!val && type == 'P') > continue; > > rc = virJSONValueObjectAppendNumberLong(obj, key, val); > } break; > > So if we "fix" QMP to reject negative values in place of large unsigned > values, we'll need to also fix libvirt to have a way to introspect the > difference and cope accordingly. (I'm thinking that we'll have to keep > the negative parsing indefinitely, and can merely improve the parser to > also parse large positive - but that we can still reject values < > INT64_MIN that would have a weird wraparound). > Currently, parsing is done without expected type information. So if the number fits in an int64, it should be a QINT, otherwise a QFLOAT (see parse_literal). With this series, it will try QUINT before QFLOAT. But what I was afraid though is that the int is sometime assigned back to a uint64. And with my series, get_uint() will fail if the number is negative int, but for compatibility reasons, I think it should cast it instead. We may want to have a qmp capability bit to change this silent casting behaviour.
diff --git a/qapi/introspect.json b/qapi/introspect.json index f6adc439bb..512a961ab3 100644 --- a/qapi/introspect.json +++ b/qapi/introspect.json @@ -125,7 +125,7 @@ # Since: 2.5 ## { 'enum': 'JSONType', - 'data': [ 'string', 'number', 'int', 'boolean', 'null', + 'data': [ 'string', 'number', 'int', 'uint', 'boolean', 'null', 'object', 'array', 'value' ] } ## diff --git a/scripts/qapi.py b/scripts/qapi.py index 9504ebd8c7..80ecc821cb 100644 --- a/scripts/qapi.py +++ b/scripts/qapi.py @@ -28,11 +28,11 @@ builtin_types = { 'int16': 'QTYPE_QINT', 'int32': 'QTYPE_QINT', 'int64': 'QTYPE_QINT', - 'uint8': 'QTYPE_QINT', - 'uint16': 'QTYPE_QINT', - 'uint32': 'QTYPE_QINT', - 'uint64': 'QTYPE_QINT', - 'size': 'QTYPE_QINT', + 'uint8': 'QTYPE_QUINT', + 'uint16': 'QTYPE_QUINT', + 'uint32': 'QTYPE_QUINT', + 'uint64': 'QTYPE_QUINT', + 'size': 'QTYPE_QUINT', 'any': None, # any QType possible, actually 'QType': 'QTYPE_QSTRING', } @@ -1093,6 +1093,7 @@ class QAPISchemaType(QAPISchemaEntity): 'string': 'QTYPE_QSTRING', 'number': 'QTYPE_QFLOAT', 'int': 'QTYPE_QINT', + 'uint': 'QTYPE_QUINT', 'boolean': 'QTYPE_QBOOL', 'object': 'QTYPE_QDICT' } @@ -1103,8 +1104,8 @@ class QAPISchemaBuiltinType(QAPISchemaType): def __init__(self, name, json_type, c_type): QAPISchemaType.__init__(self, name, None) assert not c_type or isinstance(c_type, str) - assert json_type in ('string', 'number', 'int', 'boolean', 'null', - 'value') + assert json_type in ('string', 'number', 'int', 'uint', + 'boolean', 'null', 'value') self._json_type_name = json_type self._c_type_name = c_type @@ -1519,18 +1520,19 @@ class QAPISchema(object): ('int16', 'int', 'int16_t'), ('int32', 'int', 'int32_t'), ('int64', 'int', 'int64_t'), - ('uint8', 'int', 'uint8_t'), - ('uint16', 'int', 'uint16_t'), - ('uint32', 'int', 'uint32_t'), - ('uint64', 'int', 'uint64_t'), - ('size', 'int', 'uint64_t'), + ('uint8', 'uint', 'uint8_t'), + ('uint16', 'uint', 'uint16_t'), + ('uint32', 'uint', 'uint32_t'), + ('uint64', 'uint', 'uint64_t'), + ('size', 'uint', 'uint64_t'), ('bool', 'boolean', 'bool'), ('any', 'value', 'QObject' + pointer_suffix)]: self._def_builtin_type(*t) self.the_empty_object_type = QAPISchemaObjectType('q_empty', 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', + 'qint', 'quint', 'qstring', 'qdict', 'qlist', 'qfloat', 'qbool']) self._def_entity(QAPISchemaEnumType('QType', None, qtype_values, diff --git a/include/qapi/qmp/quint.h b/include/qapi/qmp/quint.h new file mode 100644 index 0000000000..5b920ece5d --- /dev/null +++ b/include/qapi/qmp/quint.h @@ -0,0 +1,25 @@ +/* + * QUInt Module + * + * Copyright (C) 2017 Red Hat Inc. + * + * This work is licensed under the terms of the GNU LGPL, version 2.1 or later. + * See the COPYING.LIB file in the top-level directory. + */ + +#ifndef QUINT_H +#define QUINT_H + +#include "qapi/qmp/qobject.h" + +typedef struct QUInt { + QObject base; + uint64_t value; +} QUInt; + +QUInt *quint_from_uint(uint64_t value); +uint64_t quint_get_uint(const QUInt *qi); +QUInt *qobject_to_quint(const QObject *obj); +void quint_destroy_obj(QObject *obj); + +#endif /* QUINT_H */ diff --git a/include/qapi/qmp/types.h b/include/qapi/qmp/types.h index 27cfbd84e5..99a60f75d0 100644 --- a/include/qapi/qmp/types.h +++ b/include/qapi/qmp/types.h @@ -15,6 +15,7 @@ #include "qapi/qmp/qobject.h" #include "qapi/qmp/qint.h" +#include "qapi/qmp/quint.h" #include "qapi/qmp/qfloat.h" #include "qapi/qmp/qbool.h" #include "qapi/qmp/qstring.h" diff --git a/block/qapi.c b/block/qapi.c index a40922ea26..6261a49b4d 100644 --- a/block/qapi.c +++ b/block/qapi.c @@ -600,6 +600,11 @@ static void dump_qobject(fprintf_function func_fprintf, void *f, func_fprintf(f, "%" PRId64, qint_get_int(value)); break; } + case QTYPE_QUINT: { + QUInt *value = qobject_to_quint(obj); + func_fprintf(f, "%" PRIu64, quint_get_uint(value)); + break; + } case QTYPE_QSTRING: { QString *value = qobject_to_qstring(obj); func_fprintf(f, "%s", qstring_get_str(value)); diff --git a/qobject/qobject.c b/qobject/qobject.c index fe4fa10989..9bdb5e5947 100644 --- a/qobject/qobject.c +++ b/qobject/qobject.c @@ -15,6 +15,7 @@ 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_QUINT] = quint_destroy_obj, [QTYPE_QSTRING] = qstring_destroy_obj, [QTYPE_QDICT] = qdict_destroy_obj, [QTYPE_QLIST] = qlist_destroy_obj, diff --git a/qobject/quint.c b/qobject/quint.c new file mode 100644 index 0000000000..e3a7ac37c4 --- /dev/null +++ b/qobject/quint.c @@ -0,0 +1,58 @@ +/* + * QUInt Module + * + * Copyright (C) 2017 Red Hat Inc. + * + * This work is licensed under the terms of the GNU LGPL, version 2.1 or later. + * See the COPYING.LIB file in the top-level directory. + */ + +#include "qemu/osdep.h" +#include "qapi/qmp/quint.h" +#include "qapi/qmp/qobject.h" +#include "qemu-common.h" + +/** + * quint_from_uint(): Create a new QUInt from an uint64_t + * + * Return strong reference. + */ +QUInt *quint_from_uint(uint64_t value) +{ + QUInt *qi; + + qi = g_malloc(sizeof(*qi)); + qobject_init(QOBJECT(qi), QTYPE_QUINT); + qi->value = value; + + return qi; +} + +/** + * quint_get_int(): Get the stored integer + */ +uint64_t quint_get_uint(const QUInt *qi) +{ + return qi->value; +} + +/** + * qobject_to_quint(): Convert a QObject into a QUInt + */ +QUInt *qobject_to_quint(const QObject *obj) +{ + if (!obj || qobject_type(obj) != QTYPE_QUINT) { + return NULL; + } + return container_of(obj, QUInt, base); +} + +/** + * quint_destroy_obj(): Free all memory allocated by a + * QUInt object + */ +void quint_destroy_obj(QObject *obj) +{ + assert(obj != NULL); + g_free(qobject_to_quint(obj)); +} diff --git a/tests/check-qint.c b/tests/check-qint.c index b6e4555115..603aa1aa92 100644 --- a/tests/check-qint.c +++ b/tests/check-qint.c @@ -12,6 +12,7 @@ #include "qemu/osdep.h" #include "qapi/qmp/qint.h" +#include "qapi/qmp/quint.h" #include "qemu-common.h" /* @@ -73,6 +74,58 @@ static void qobject_to_qint_test(void) QDECREF(qi); } +static void quint_from_uint_test(void) +{ + QUInt *qu; + const unsigned value = -42; + + qu = quint_from_uint(value); + g_assert(qu != NULL); + g_assert(qu->value == value); + g_assert(qu->base.refcnt == 1); + g_assert(qobject_type(QOBJECT(qu)) == QTYPE_QUINT); + + g_free(qu); +} + +static void quint_destroy_test(void) +{ + QUInt *qu = quint_from_uint(0); + QDECREF(qu); +} + +static void quint_from_uint64_test(void) +{ + QUInt *qu; + const uint64_t value = 0x1234567890abcdefLL; + + qu = quint_from_uint(value); + g_assert((uint64_t) qu->value == value); + + QDECREF(qu); +} + +static void quint_get_uint_test(void) +{ + QUInt *qu; + const unsigned value = 123456; + + qu = quint_from_uint(value); + g_assert(quint_get_uint(qu) == value); + + QDECREF(qu); +} + +static void qobject_to_quint_test(void) +{ + QUInt *qu; + + qu = quint_from_uint(0); + g_assert(qobject_to_quint(QOBJECT(qu)) == qu); + + QDECREF(qu); +} + int main(int argc, char **argv) { g_test_init(&argc, &argv, NULL); @@ -83,5 +136,11 @@ int main(int argc, char **argv) g_test_add_func("/public/get_int", qint_get_int_test); g_test_add_func("/public/to_qint", qobject_to_qint_test); + g_test_add_func("/public/from_uint", quint_from_uint_test); + g_test_add_func("/public/uint_destroy", quint_destroy_test); + g_test_add_func("/public/from_uint64", quint_from_uint64_test); + g_test_add_func("/public/get_uint", quint_get_uint_test); + g_test_add_func("/public/to_quint", qobject_to_quint_test); + return g_test_run(); } diff --git a/qobject/Makefile.objs b/qobject/Makefile.objs index bed55084bb..1f7b95552f 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 qint.o quint.o qstring.o qdict.o qlist.o qfloat.o qbool.o util-obj-y += qjson.o qobject.o json-lexer.o json-streamer.o json-parser.o diff --git a/tests/qapi-schema/comments.out b/tests/qapi-schema/comments.out index a962fb2d2e..42ea87a1a1 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', 'qint', 'quint', 'qstring', 'qdict', 'qlist', 'qfloat', 'qbool'] prefix QTYPE enum Status ['good', 'bad', 'ugly'] object q_empty diff --git a/tests/qapi-schema/empty.out b/tests/qapi-schema/empty.out index 8a5b034424..5dcccb1d55 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', 'qint', 'quint', 'qstring', 'qdict', 'qlist', 'qfloat', 'qbool'] prefix QTYPE object q_empty diff --git a/tests/qapi-schema/event-case.out b/tests/qapi-schema/event-case.out index 2865714ad5..3464bd30f6 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', 'qint', 'quint', 'qstring', 'qdict', 'qlist', 'qfloat', '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 69fc908e68..a81ef8cd34 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', 'qint', 'quint', 'qstring', 'qdict', 'qlist', 'qfloat', '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 a962fb2d2e..42ea87a1a1 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', 'qint', 'quint', 'qstring', 'qdict', 'qlist', 'qfloat', '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 a962fb2d2e..42ea87a1a1 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', 'qint', 'quint', 'qstring', 'qdict', 'qlist', 'qfloat', '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 a962fb2d2e..42ea87a1a1 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', 'qint', 'quint', 'qstring', 'qdict', 'qlist', 'qfloat', '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 285d052257..b158bcd6eb 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', 'qint', 'quint', 'qstring', 'qdict', 'qlist', 'qfloat', '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 bc8d496ff4..690460658c 100644 --- a/tests/qapi-schema/qapi-schema-test.out +++ b/tests/qapi-schema/qapi-schema-test.out @@ -54,7 +54,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', 'qint', 'quint', 'qstring', 'qdict', 'qlist', 'qfloat', 'qbool'] prefix QTYPE object TestStruct member integer: int optional=False
The type is not used at all yet. Add some tests to exercice it. Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com> --- qapi/introspect.json | 2 +- scripts/qapi.py | 28 ++++++++------- include/qapi/qmp/quint.h | 25 ++++++++++++++ include/qapi/qmp/types.h | 1 + block/qapi.c | 5 +++ qobject/qobject.c | 1 + qobject/quint.c | 58 +++++++++++++++++++++++++++++++ tests/check-qint.c | 59 ++++++++++++++++++++++++++++++++ qobject/Makefile.objs | 2 +- tests/qapi-schema/comments.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 +- 18 files changed, 175 insertions(+), 24 deletions(-) create mode 100644 include/qapi/qmp/quint.h create mode 100644 qobject/quint.c