From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mailman by lists.gnu.org with tmda-scanned (Exim 4.43) id 1MRsEK-0006uM-Gj for qemu-devel@nongnu.org; Fri, 17 Jul 2009 14:32:24 -0400 Received: from exim by lists.gnu.org with spam-scanned (Exim 4.43) id 1MRsEJ-0006tb-R7 for qemu-devel@nongnu.org; Fri, 17 Jul 2009 14:32:24 -0400 Received: from [199.232.76.173] (port=48300 helo=monty-python.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1MRsEJ-0006tJ-KT for qemu-devel@nongnu.org; Fri, 17 Jul 2009 14:32:23 -0400 Received: from fg-out-1718.google.com ([72.14.220.154]:7763) by monty-python.gnu.org with esmtp (Exim 4.60) (envelope-from ) id 1MRsEJ-0004RT-4h for qemu-devel@nongnu.org; Fri, 17 Jul 2009 14:32:23 -0400 Received: by fg-out-1718.google.com with SMTP id l27so259240fgb.8 for ; Fri, 17 Jul 2009 11:32:22 -0700 (PDT) MIME-Version: 1.0 In-Reply-To: <4A60BEEF.2050601@us.ibm.com> References: <1247841685-18495-1-git-send-email-aliguori@us.ibm.com> <1247841685-18495-3-git-send-email-aliguori@us.ibm.com> <200907171833.34167.paul@codesourcery.com> <4A60BEEF.2050601@us.ibm.com> From: Blue Swirl Date: Fri, 17 Jul 2009 21:32:02 +0300 Message-ID: Subject: Re: [Qemu-devel] [PATCH 2/2] Introduce macro for defining qdev properties Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Anthony Liguori Cc: Gerd Hoffman , Paul Brook , qemu-devel@nongnu.org On Fri, Jul 17, 2009 at 9:11 PM, Anthony Liguori wrote= : > Paul Brook wrote: >> >> I think I agree. Using GCC extensions for error checking (e.g. DO_UPCAST= ) >> or performance (__builtin_clz) is fine, but I'm a reluctant to rely on i= t >> for correct operation. >> > > Practically speaking, we've never supported anything but GCC and I doubt = we > ever will. =C2=A0In this case, it's an important part of something I'm tr= ying to > fix about the current property system. > > It seems very brittle to me. =C2=A0You have to specify a type in both the= state > structure and in the property definition. =C2=A0Those two things are very= , very > far apart in the code. =C2=A0Right now, the rules about type compatible a= re ill > defined which makes it more likely to break beyond simple mistakes. =C2= =A0For > instance, uint32 is used for uint32_t, int32_t, and int. =C2=A0That seems= odd. > > I also don't like the fact that we mix field type information with displa= y > information. =C2=A0I haven't thought about the best solution to this but = I think > it's either introducing new struct types or adding an optional decorator > parameter. > > The system I'm aiming for looks like this: > > typedef struct { > =C2=A0SysBusDevice parent; > > =C2=A0/* public */ > =C2=A0uint32_t queue_depth; > =C2=A0uint32_t tx_mitigation_delay; > =C2=A0CharDriverState *chr; > > =C2=A0/* private */ > =C2=A0... > } MyDeviceState; > > static Property my_device_properties[] =3D { > =C2=A0QDEV_PROP(MyDeviceState, queue_depth), > =C2=A0QDEV_PROP(MyDeviceState, tx_mitigation_delay), > =C2=A0QDEV_PROP(MyDeviceState, chr), > =C2=A0{} > }; > > Where there's a connection between properties and device state fields and > there's no duplicate type information. =C2=A0That means that for the most= part, > the rules of type compatible can be ignored by most users. > > I'd like to see most uses of QDEV_PROP_NAME eliminated by renaming variab= les > and accepting '-' in place of '_'. =C2=A0We'll always need a way to accep= t > default values. > > I'm not sure how to do this without GCC extensions. =C2=A0We could potent= ially > add macro decorators and use a sparse-like tool to extract property lists > automatically from device state. Then there is the template way: typedef struct MyDeviceState { #define PROP(type, name) type name; #define C_TYPE #include "mydevice_props.hx" } MyDeviceState; #undef PROP #undef C_TYPE static Property my_device_properties[] =3D { #define PROP(type, name) glue(QDEV_PROP_, type)(MyDeviceState, name), #include "mydevice_props.hx" {} }; Where mydevice_props.hx contains: #ifdef C_TYPE #define I32 uint32_t #define I64 uint64_t #define CHRDEV CharDriverState * #endif PROP(I32, queue_depth) PROP(I32, tx_mitigation_delay) PROP(CHRDEV, chr)