Re: [Qemu-devel] [PATCH] test: QOM interface casting

[Igor Mammedov <imammedo@redhat.com>]

On Fri, 20 Dec 2013 14:58:07 +0100
Andreas Färber <afaerber@suse.de> wrote:

> Am 20.12.2013 14:26, schrieb Igor Mammedov:
> > Add basic regression testing for QOM Interface usage.
> > 
> > Signed-off-by: Igor Mammedov <imammedo@redhat.com>
> > ---
> > v2:
> >  - Peter Crosthwaite <peter.crosthwaite@xilinx.com>
> >    * s/Parent/parent_obj/
> >    * s/parent/parent_class/
> >  - Peter Maydell <peter.maydell@linaro.org>
> >    * s/interfacei/interface/
> >    * tests/Makefile split too long line
> >  - Andreas Färber <afaerber@suse.de>
> >    * 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 <imammedo@redhat.com>
> > + *
> > + * 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 ...

> 
> > +
> > +    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
>