| Message ID | 20201123194818.2773508-11-ehabkost@redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series | qom: Use qlit to represent property defaults | expand |
Eduardo Habkost <ehabkost@redhat.com> writes: > Add two new macros to support other types of QNums: > QLIT_QNUM_UINT, and QLIT_QNUM_DOUBLE, and include them > in the qlit_equal_qobject_test() test case. > > Signed-off-by: Eduardo Habkost <ehabkost@redhat.com> > --- > Changes v2 -> v3: > * QLIT_QNUM macro doesn't exist anymore > * Addition of the QNumValue field to QLitObject is > now in a separate patch ("qlit: Use QNumValue to represent QNums") > * check-qjson test case changes dropped. > Instead, I'm only extending the qlit_equal_qobject_test() test > case. > > Changes v1 -> v2: > * Coding style fix at qlit_equal_qobject() > --- > include/qapi/qmp/qlit.h | 4 ++++ > tests/check-qlit.c | 5 +++++ > 2 files changed, 9 insertions(+) > > diff --git a/include/qapi/qmp/qlit.h b/include/qapi/qmp/qlit.h > index a240cdd299..a2881b7f42 100644 > --- a/include/qapi/qmp/qlit.h > +++ b/include/qapi/qmp/qlit.h > @@ -42,6 +42,10 @@ struct QLitDictEntry { > { .type = QTYPE_QBOOL, .value.qbool = (val) } > #define QLIT_QNUM_INT(val) \ > { .type = QTYPE_QNUM, .value.qnum = QNUM_VAL_INT(val) } > +#define QLIT_QNUM_UINT(val) \ > + { .type = QTYPE_QNUM, .value.qnum = QNUM_VAL_UINT(val) } > +#define QLIT_QNUM_DOUBLE(val) \ > + { .type = QTYPE_QNUM, .value.qnum = QNUM_VAL_DOUBLE(val) } > #define QLIT_QSTR(val) \ > { .type = QTYPE_QSTRING, .value.qstr = (val) } > #define QLIT_QDICT(val) \ > diff --git a/tests/check-qlit.c b/tests/check-qlit.c > index 5a9260b93f..31e90f8965 100644 > --- a/tests/check-qlit.c > +++ b/tests/check-qlit.c > @@ -58,6 +58,11 @@ static void qlit_equal_qobject_test(void) > QLIT_QNUM_INT(1), > QLIT_QNUM_INT(INT64_MIN), > QLIT_QNUM_INT(INT64_MAX), > + QLIT_QNUM_UINT(UINT64_MAX), > + /* Larger than UINT64_MAX: */ > + QLIT_QNUM_DOUBLE(18446744073709552e3), > + /* Smaller than INT64_MIN: */ > + QLIT_QNUM_DOUBLE(-92233720368547758e2), Why "larger than UINT64_MAX" and "smaller than INT64_MIN"? > QLIT_QSTR(""), > QLIT_QSTR("foo"), > qlit,
On 24/11/20 10:55, Markus Armbruster wrote: >> + /* Larger than UINT64_MAX: */ >> + QLIT_QNUM_DOUBLE(18446744073709552e3), >> + /* Smaller than INT64_MIN: */ >> + QLIT_QNUM_DOUBLE(-92233720368547758e2), > Why "larger than UINT64_MAX" and "smaller than INT64_MIN"? > I guess the point is to test values that are only representable as a double, so (double)((uint64_t)INT64_MAX+1) wouldn't be very useful for that: as the expression shows, it would not be a QNUM_VAL_INT but it would be representable as QNUM_VAL_UINT. So these are the cases that matter the most, even though -1, 0 and INT64_MAX+1 could be nice to have. Paolo
Paolo Bonzini <pbonzini@redhat.com> writes: > On 24/11/20 10:55, Markus Armbruster wrote: >>> + /* Larger than UINT64_MAX: */ >>> + QLIT_QNUM_DOUBLE(18446744073709552e3), >>> + /* Smaller than INT64_MIN: */ >>> + QLIT_QNUM_DOUBLE(-92233720368547758e2), >> Why "larger than UINT64_MAX" and "smaller than INT64_MIN"? >> > > I guess the point is to test values that are only representable as a > double, so (double)((uint64_t)INT64_MAX+1) wouldn't be very useful for > that: as the expression shows, it would not be a QNUM_VAL_INT but it > would be representable as QNUM_VAL_UINT. > > So these are the cases that matter the most, even though -1, 0 and > INT64_MAX+1 could be nice to have. qnum_is_equal()'s contract: * Doubles are never considered equal to integers.
On Tue, Nov 24, 2020 at 01:22:02PM +0100, Markus Armbruster wrote: > Paolo Bonzini <pbonzini@redhat.com> writes: > > > On 24/11/20 10:55, Markus Armbruster wrote: > >>> + /* Larger than UINT64_MAX: */ > >>> + QLIT_QNUM_DOUBLE(18446744073709552e3), > >>> + /* Smaller than INT64_MIN: */ > >>> + QLIT_QNUM_DOUBLE(-92233720368547758e2), > >> Why "larger than UINT64_MAX" and "smaller than INT64_MIN"? > >> > > > > I guess the point is to test values that are only representable as a > > double, so (double)((uint64_t)INT64_MAX+1) wouldn't be very useful for > > that: as the expression shows, it would not be a QNUM_VAL_INT but it > > would be representable as QNUM_VAL_UINT. > > > > So these are the cases that matter the most, even though -1, 0 and > > INT64_MAX+1 could be nice to have. > > qnum_is_equal()'s contract: > > * Doubles are never considered equal to integers. If that's part of the contract, it would be OK to include 0.0, -1.0, 1.0, INT64_MAX+1 in the list. I incorrectly assumed qnum_is_equal(qnum_from_int(0), qnum_from_double(0.0)) was undefined. However, if we really care about test coverage of qnum_is_equal(), we probably should be extending check-qnum.c, not check-qlit.c.
diff --git a/include/qapi/qmp/qlit.h b/include/qapi/qmp/qlit.h index a240cdd299..a2881b7f42 100644 --- a/include/qapi/qmp/qlit.h +++ b/include/qapi/qmp/qlit.h @@ -42,6 +42,10 @@ struct QLitDictEntry { { .type = QTYPE_QBOOL, .value.qbool = (val) } #define QLIT_QNUM_INT(val) \ { .type = QTYPE_QNUM, .value.qnum = QNUM_VAL_INT(val) } +#define QLIT_QNUM_UINT(val) \ + { .type = QTYPE_QNUM, .value.qnum = QNUM_VAL_UINT(val) } +#define QLIT_QNUM_DOUBLE(val) \ + { .type = QTYPE_QNUM, .value.qnum = QNUM_VAL_DOUBLE(val) } #define QLIT_QSTR(val) \ { .type = QTYPE_QSTRING, .value.qstr = (val) } #define QLIT_QDICT(val) \ diff --git a/tests/check-qlit.c b/tests/check-qlit.c index 5a9260b93f..31e90f8965 100644 --- a/tests/check-qlit.c +++ b/tests/check-qlit.c @@ -58,6 +58,11 @@ static void qlit_equal_qobject_test(void) QLIT_QNUM_INT(1), QLIT_QNUM_INT(INT64_MIN), QLIT_QNUM_INT(INT64_MAX), + QLIT_QNUM_UINT(UINT64_MAX), + /* Larger than UINT64_MAX: */ + QLIT_QNUM_DOUBLE(18446744073709552e3), + /* Smaller than INT64_MIN: */ + QLIT_QNUM_DOUBLE(-92233720368547758e2), QLIT_QSTR(""), QLIT_QSTR("foo"), qlit,
Add two new macros to support other types of QNums: QLIT_QNUM_UINT, and QLIT_QNUM_DOUBLE, and include them in the qlit_equal_qobject_test() test case. Signed-off-by: Eduardo Habkost <ehabkost@redhat.com> --- Changes v2 -> v3: * QLIT_QNUM macro doesn't exist anymore * Addition of the QNumValue field to QLitObject is now in a separate patch ("qlit: Use QNumValue to represent QNums") * check-qjson test case changes dropped. Instead, I'm only extending the qlit_equal_qobject_test() test case. Changes v1 -> v2: * Coding style fix at qlit_equal_qobject() --- include/qapi/qmp/qlit.h | 4 ++++ tests/check-qlit.c | 5 +++++ 2 files changed, 9 insertions(+)