From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:43522) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZYs45-00024Z-Aa for qemu-devel@nongnu.org; Mon, 07 Sep 2015 04:46:14 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1ZYs41-0002xo-Li for qemu-devel@nongnu.org; Mon, 07 Sep 2015 04:46:13 -0400 Received: from mx1.redhat.com ([209.132.183.28]:60152) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZYs41-0002xi-Ga for qemu-devel@nongnu.org; Mon, 07 Sep 2015 04:46:09 -0400 Date: Mon, 7 Sep 2015 09:46:04 +0100 From: "Daniel P. Berrange" Message-ID: <20150907084604.GA29882@redhat.com> References: <1440590594-5514-1-git-send-email-berrange@redhat.com> <1440590594-5514-2-git-send-email-berrange@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [PATCH RFC 1/7] qom: allow properties to be registered against classes Reply-To: "Daniel P. Berrange" List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: =?utf-8?Q?Marc-Andr=C3=A9?= Lureau Cc: QEMU , Andreas =?utf-8?Q?F=C3=A4rber?= , Markus Armbruster On Fri, Sep 04, 2015 at 11:38:06PM +0200, Marc-Andr=C3=A9 Lureau wrote: > Hi >=20 > On Wed, Aug 26, 2015 at 2:03 PM, Daniel P. Berrange wrote: > > +ObjectProperty * > > +object_class_property_add(ObjectClass *klass, > > + const char *name, > > + const char *type, > > + ObjectPropertyAccessor *get, > > + ObjectPropertyAccessor *set, > > + ObjectPropertyRelease *release, > > + void *opaque, > > + Error **errp) > > +{ > > + ObjectProperty *prop; > > + size_t name_len =3D strlen(name); > > + > > + if (name_len >=3D 3 && !memcmp(name + name_len - 3, "[*]", 4)) { > > + int i; > > + ObjectProperty *ret; > > + char *name_no_array =3D g_strdup(name); > > + >=20 > I question the need for dynamic/array property name registered in > classes. What would be more useful is an array property instead. It > would help to introspect classes for dynamic "children[*]" case. > object_property_add_child() could verify/check against the class > declaration, and grow the instance properties list (like it does now, > but it would be only for instances of children[] items). On > introspection of classes, the class "children[*]" property would be > visible, but would be hidden when introspecting the instance, and you > wouldn't be able to lookup that "array" property. >=20 > It seems relatively straightforward to deal with the link<> case, by > storing the offset of the "child" pointer. This seems fine if limited > to a single link<> (it should probably check the prop is not of the > name[*] style already), ex: > https://gist.github.com/elmarco/905241b683fb9c5f2a08 >=20 > Your patches looks good to me in general but object_property_del() > should be fixed, since the prop find may belong to the class. Actually I skipped object_property_del() intentionally. Classes should be immutable once defined, so deleting a property from a class would not be appropriate. Regards, Daniel --=20 |: http://berrange.com -o- http://www.flickr.com/photos/dberrange= / :| |: http://libvirt.org -o- http://virt-manager.or= g :| |: http://autobuild.org -o- http://search.cpan.org/~danberr= / :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vn= c :|