[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH] test: QOM interface casting
From: |
Peter Crosthwaite |
Subject: |
Re: [Qemu-devel] [PATCH] test: QOM interface casting |
Date: |
Sat, 21 Dec 2013 07:27:04 +1000 |
On Sat, Dec 21, 2013 at 12:08 AM, Igor Mammedov <address@hidden> wrote:
> On Fri, 20 Dec 2013 14:58:07 +0100
> Andreas Färber <address@hidden> wrote:
>
>> Am 20.12.2013 14:26, schrieb Igor Mammedov:
>> > Add basic regression testing for QOM Interface usage.
>> >
>> > Signed-off-by: Igor Mammedov <address@hidden>
>> > ---
>> > v2:
>> > - Peter Crosthwaite <address@hidden>
>> > * s/Parent/parent_obj/
>> > * s/parent/parent_class/
>> > - Peter Maydell <address@hidden>
>> > * s/interfacei/interface/
>> > * tests/Makefile split too long line
>> > - Andreas Färber <address@hidden>
>> > * consolidate QOM core object files in dedicated variable
>> > * add SoB and commit message
>> > ---
>> > tests/Makefile | 6 ++-
>> > tests/check-qom-interface.c | 102
>> > +++++++++++++++++++++++++++++++++++++++++++
>> > 2 files changed, 107 insertions(+), 1 deletions(-)
>> > create mode 100644 tests/check-qom-interface.c
>> >
>> > diff --git a/tests/Makefile b/tests/Makefile
>> > index 379cdd9..89f8eff 100644
>> > --- a/tests/Makefile
>> > +++ b/tests/Makefile
>> > @@ -52,6 +52,8 @@ check-unit-y += tests/test-int128$(EXESUF)
>> > gcov-files-test-int128-y =
>> > check-unit-y += tests/test-bitops$(EXESUF)
>> > check-unit-y += tests/test-qdev-global-props$(EXESUF)
>> > +check-unit-y = tests/check-qom-interface$(EXESUF)
>>
>> Surely needs to be +=.
> fixed.
>>
>> > +gcov-files-check-qdict-y = object/object.c
>>
>> Copy&paste? ("qdict")
>> Also "qom/object.c".
> fixed
>
>>
>> >
>> > check-block-$(CONFIG_POSIX) += tests/qemu-iotests-quick.sh
>> >
>> > @@ -137,6 +139,7 @@ test-qapi-obj-y = tests/test-qapi-visit.o
>> > tests/test-qapi-types.o
>> >
>> > $(test-obj-y): QEMU_INCLUDES += -Itests
>> > QEMU_CFLAGS += -I$(SRC_PATH)/tests
>> > +qom-core-obj = qom/object.o qom/qom-qobject.o qom/container.o
>> >
>> > tests/test-x86-cpuid.o: QEMU_INCLUDES += -I$(SRC_PATH)/target-i386
>> >
>> > @@ -146,6 +149,7 @@ tests/check-qdict$(EXESUF): tests/check-qdict.o
>> > libqemuutil.a
>> > tests/check-qlist$(EXESUF): tests/check-qlist.o libqemuutil.a
>> > tests/check-qfloat$(EXESUF): tests/check-qfloat.o libqemuutil.a
>> > tests/check-qjson$(EXESUF): tests/check-qjson.o libqemuutil.a
>> > libqemustub.a
>> > +tests/check-qom-interface$(EXESUF): tests/check-qom-interface.o
>> > $(qom-core-obj) libqemuutil.a
>> > tests/test-coroutine$(EXESUF): tests/test-coroutine.o $(block-obj-y)
>> > libqemuutil.a libqemustub.a
>> > tests/test-aio$(EXESUF): tests/test-aio.o $(block-obj-y) libqemuutil.a
>> > libqemustub.a
>> > tests/test-throttle$(EXESUF): tests/test-throttle.o $(block-obj-y)
>> > libqemuutil.a libqemustub.a
>> > @@ -159,7 +163,7 @@ tests/test-int128$(EXESUF): tests/test-int128.o
>> > tests/test-qdev-global-props$(EXESUF): tests/test-qdev-global-props.o \
>> > hw/core/qdev.o hw/core/qdev-properties.o \
>> > hw/core/irq.o \
>> > - qom/object.o qom/container.o qom/qom-qobject.o \
>> > + $(qom-core-obj) \
>> > $(test-qapi-obj-y) \
>> > libqemuutil.a libqemustub.a
>> >
>> > diff --git a/tests/check-qom-interface.c b/tests/check-qom-interface.c
>> > new file mode 100644
>> > index 0000000..8d7c1f8
>> > --- /dev/null
>> > +++ b/tests/check-qom-interface.c
>> > @@ -0,0 +1,102 @@
>> > +/*
>> > + * OQM interface test.
>> > + *
>> > + * Copyright (C) 2013 Red Hat Inc.
>> > + *
>> > + * Authors:
>> > + * Igor Mammedov <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 <glib.h>
>> > +
>> > +#include "qom/object.h"
>> > +#include "qemu/module.h"
>> > +
>> > +
>> > +#define TYPE_TEST_IF "test-interface"
>> > +#define TEST_IF_CLASS(klass) \
>> > + OBJECT_CLASS_CHECK(TestIfClass, (klass), TYPE_TEST_IF)
>> > +#define TEST_IF_GET_CLASS(obj) \
>> > + OBJECT_GET_CLASS(TestIfClass, (obj), TYPE_TEST_IF)
>> > +#define TEST_IF(obj) \
>> > + INTERFACE_CHECK(TestIf, (obj), TYPE_TEST_IF)
>> > +
>> > +typedef struct TestIf {
>> > + Object parent_obj;
>> > +} TestIf;
>> > +
>> > +typedef struct TestIfClass {
>> > + InterfaceClass parent_class;
>> > +
>> > + uint32_t test;
>> > +} TestIfClass;
>> > +
>> > +static const TypeInfo test_if_info = {
>> > + .name = TYPE_TEST_IF,
>> > + .parent = TYPE_INTERFACE,
>>
>> TestIf is not assigned here - no bug but let's set a consistent example.
> What do you mean?
>
>>
>> > + .class_size = sizeof(TestIfClass),
>> > +};
>> > +
>> > +#define PATTERN 0xFAFBFCFD
>> > +static void test_class_init(ObjectClass *oc, void *data)
>> > +{
>> > + TestIfClass *tc = TEST_IF_CLASS(oc);
>> > +
>> > + g_assert(tc);
>> > + tc->test = PATTERN;
>> > +}
>> > +
>> > +#define TYPE_DIRECT_IMPL "direct-impl"
>> > +static const TypeInfo direct_impl_info = {
>> > + .name = TYPE_DIRECT_IMPL,
>> > + .parent = TYPE_OBJECT,
>> > + .class_init = test_class_init,
>> > + .interfaces = (InterfaceInfo[]) {
>> > + { TYPE_TEST_IF },
>> > + { }
>> > + }
>> > +};
>> > +
>> > +#define TYPE_INTERMEDIATE_IMPL "intermediate-impl"
>>
>> I'd insert a white line here and above.
> sure
>
>>
>> > +static const TypeInfo intermediate_impl_info = {
>> > + .name = TYPE_INTERMEDIATE_IMPL,
>> > + .parent = TYPE_DIRECT_IMPL,
>> > +};
>> > +
>> > +static void test_interface_impl(const char *type)
>> > +{
>> > + Object *obj = object_new(type);
>> > + TestIf *iobj = TEST_IF(obj);
>> > + TestIfClass *ico = TEST_IF_GET_CLASS(iobj);
>>
>> What's the o in ico for BTW?
> InterfaceClassObject ...
>
This seems strange to me. In some other code I rmbr just using lower
case acronym versions of the type to keep it all consistent. This
means you could use "tic" for TestIfClass.
Regards,
Peter
>>
>> > +
>> > + g_assert(iobj);
>> > + g_assert(ico->test == PATTERN);
>> > +}
>> > +
>> > +static void interface_direct_test(void)
>> > +{
>> > + test_interface_impl(TYPE_DIRECT_IMPL);
>> > +}
>> > +
>> > +static void interface_intermediate_test(void)
>> > +{
>> > + test_interface_impl(TYPE_INTERMEDIATE_IMPL);
>> > +}
>> > +
>> > +int main(int argc, char **argv)
>> > +{
>> > + g_test_init(&argc, &argv, NULL);
>> > +
>> > + module_call_init(MODULE_INIT_QOM);
>> > + type_register_static(&test_if_info);
>> > + type_register_static(&direct_impl_info);
>> > + type_register_static(&intermediate_impl_info);
>> > +
>> > + g_test_add_func("/interface/direct_impl", interface_direct_test);
>> > + g_test_add_func("/interface/intermediate_impl",
>>
>> Would it make sense to call it /qom/interface/... or /qom-interface/...?
> sure, I'll amend it.
>
>> Doesn't really matter though as long as it's unique, I guess.
>>
>> > + interface_intermediate_test);
>> > +
>> > + return g_test_run();
>> > +}
>>
>> Regards,
>> Andreas
>>
>
>