From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:55547) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1SF2sm-0008Gy-5d for qemu-devel@nongnu.org; Tue, 03 Apr 2012 08:30:45 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1SF2se-0007hR-33 for qemu-devel@nongnu.org; Tue, 03 Apr 2012 08:30:43 -0400 Received: from cantor2.suse.de ([195.135.220.15]:44384 helo=mx2.suse.de) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1SF2sd-0007gn-Ps for qemu-devel@nongnu.org; Tue, 03 Apr 2012 08:30:36 -0400 Message-ID: <4F7AED5F.6000200@suse.de> Date: Tue, 03 Apr 2012 14:30:23 +0200 From: =?ISO-8859-15?Q?Andreas_F=E4rber?= MIME-Version: 1.0 References: <1333451753-3550-1-git-send-email-pbonzini@redhat.com> <1333451753-3550-5-git-send-email-pbonzini@redhat.com> In-Reply-To: <1333451753-3550-5-git-send-email-pbonzini@redhat.com> Content-Type: text/plain; charset=ISO-8859-15 Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [PATCH 04/25] qom: make Object a type List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Paolo Bonzini Cc: aliguori@us.ibm.com, qemu-devel@nongnu.org Am 03.04.2012 13:15, schrieb Paolo Bonzini: > Right now the base Object class has a special NULL type. Change this s= o > that we will be able to add class_init and class_base_init callbacks. > To do this, remove some special casing of ObjectClass that is not reall= y > necessary. While the patch itself looks good, we should be aware that this changes semantics: Before this patch, lack of .parent is identical to .parent =3D TYPE_OBJECT; with this patch that would become another base class. Should be mentioned in the commit message and all TypeInfos need to be reviewed. I don't think accidentally growing base classes is a good idea. Maybe whitelist valid base classes in type_register_static() and abort otherwise? Then at least we'd catch it by just running the executables. >=20 > Signed-off-by: Paolo Bonzini > --- > include/qemu/object.h | 2 +- > qom/object.c | 59 +++++++++++++++++++++++++----------------= -------- > 2 files changed, 31 insertions(+), 30 deletions(-) >=20 > diff --git a/include/qemu/object.h b/include/qemu/object.h > index ccaea7d..22f646d 100644 > --- a/include/qemu/object.h > +++ b/include/qemu/object.h > @@ -33,7 +33,7 @@ typedef struct TypeInfo TypeInfo; > typedef struct InterfaceClass InterfaceClass; > typedef struct InterfaceInfo InterfaceInfo; > =20 > -#define TYPE_OBJECT NULL > +#define TYPE_OBJECT "object" > =20 > /** > * SECTION:object.h > diff --git a/qom/object.c b/qom/object.c > index 6ff1c19..585619d 100644 > --- a/qom/object.c > +++ b/qom/object.c > @@ -210,7 +210,7 @@ static void type_class_interface_init(TypeImpl *ti,= InterfaceImpl *iface) > =20 > static void type_initialize(TypeImpl *ti) > { > - size_t class_size =3D sizeof(ObjectClass); > + TypeImpl *parent; > int i; > =20 > if (ti->class) { > @@ -221,30 +221,24 @@ static void type_initialize(TypeImpl *ti) > ti->instance_size =3D type_object_get_size(ti); > =20 > ti->class =3D g_malloc0(ti->class_size); > - ti->class->type =3D ti; > - > - if (type_has_parent(ti)) { > - TypeImpl *parent =3D type_get_parent(ti); > =20 > + parent =3D type_get_parent(ti); > + if (parent) { > type_initialize(parent); > =20 > - class_size =3D parent->class_size; > g_assert(parent->class_size <=3D ti->class_size); > + memcpy(ti->class, parent->class, parent->class_size); > + } > =20 > - memcpy((void *)ti->class + sizeof(ObjectClass), > - (void *)parent->class + sizeof(ObjectClass), > - parent->class_size - sizeof(ObjectClass)); Is this really safe? Before we would only copy the contents of derived classes. Missing some explanations IMO. > + ti->class->type =3D ti; > =20 > - while (parent) { > - if (parent->class_base_init) { > - parent->class_base_init(ti->class, ti->class_data); > - } > - parent =3D type_get_parent(parent); > + while (parent) { > + if (parent->class_base_init) { > + parent->class_base_init(ti->class, ti->class_data); > } > + parent =3D type_get_parent(parent); > } > =20 > - memset((void *)ti->class + class_size, 0, ti->class_size - class_s= ize); > - > for (i =3D 0; i < ti->num_interfaces; i++) { > type_class_interface_init(ti, &ti->interfaces[i]); > } > @@ -467,19 +461,6 @@ Object *object_dynamic_cast(Object *obj, const cha= r *typename) > } > =20 > =20 > -static void register_types(void) > -{ > - static TypeInfo interface_info =3D { > - .name =3D TYPE_INTERFACE, > - .instance_size =3D sizeof(Interface), > - .abstract =3D true, > - }; > - > - type_interface =3D type_register_static(&interface_info); > -} > - > -type_init(register_types) > - > Object *object_dynamic_cast_assert(Object *obj, const char *typename) > { > Object *inst; > @@ -1233,3 +1214,23 @@ void object_property_add_str(Object *obj, const = char *name, > property_release_str, > prop, errp); > } > + > +static void register_types(void) > +{ > + static TypeInfo interface_info =3D { > + .name =3D TYPE_INTERFACE, > + .instance_size =3D sizeof(Interface), > + .abstract =3D true, > + }; > + > + static TypeInfo object_info =3D { > + .name =3D TYPE_OBJECT, > + .instance_size =3D sizeof(Object), > + .abstract =3D true, > + }; > + > + type_interface =3D type_register_static(&interface_info); > + type_register_static(&object_info); > +} > + > +type_init(register_types) Thanks for moving these down. Is there a reason not to make them static const just before register_types()? Andreas --=20 SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 N=FCrnberg, Germany GF: Jeff Hawn, Jennifer Guild, Felix Imend=F6rffer; HRB 16746 AG N=FCrnbe= rg