On Mon, Dec 17, 2012 at 06:42:58PM -0600, Anthony Liguori wrote: > "Michael S. Tsirkin" <m...@redhat.com> writes: > > > On Mon, Dec 17, 2012 at 11:59:15PM +0100, Andreas Färber wrote: > >> Am 17.12.2012 22:40, schrieb Michael S. Tsirkin: > >> > Move bindings from opaque to DeviceState. > >> > This gives us better type safety with no performance cost. > >> > Add macros to make future QOM work easier, document > >> > which ones are data-path sensitive. > >> > > >> > Signed-off-by: Michael S. Tsirkin <m...@redhat.com> > >> > --- > >> > > >> > Changes from v1: > >> > - Address comment by Anreas Färber: wrap container_of > >> > macros to make future QOM work easier > >> > - make a couple of bindings that v1 missed typesafe: > >> > virtio doesn't use any void * now > >> > > >> > diff --git a/hw/s390-virtio-bus.c b/hw/s390-virtio-bus.c > >> > index e0ac2d1..8c693b4 100644 > >> > --- a/hw/s390-virtio-bus.c > >> > +++ b/hw/s390-virtio-bus.c > >> > @@ -137,7 +137,7 @@ static int s390_virtio_device_init(VirtIOS390Device > >> > *dev, VirtIODevice *vdev) > >> > > >> > bus->dev_offs += dev_len; > >> > > >> > - virtio_bind_device(vdev, &virtio_s390_bindings, dev); > >> > + virtio_bind_device(vdev, &virtio_s390_bindings, > >> > VIRTIO_S390_TO_QDEV(dev)); > >> > >> DEVICE(dev) exists for exactly that purpose, and device init is > >> certainly no hot path. Please don't reinvent the wheel for virtio. > > > > OK. > > Though my beef with DEVICE is that it ignores the type > > passed in completely. You can give it int * and it will > > happily cast to devicestate. Your only hope is to > > catch the error at runtime. > > That's a feature. DEVICE can do upcasting and downcasting. There's no > way to do compile time checking of upcasting when > > > It would be better if DEVICE got the name of the > > qdev field, then we could check it's actually DeviceState > > before casting. Yes it would mean a bit of churn if you rename the > > field but it's very rare and trivial to change by a regexp. > > No, it would be much, much worse. You shouldn't have to know what the > layout of the structure is to convert between types.
Still I'm pointing out the problems, they are real. Illegal code like DEVICE("foobar") compiles fine and it shouldn't. -- MST