From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mailman by lists.gnu.org with tmda-scanned (Exim 4.43) id 1MPM01-00080j-27 for qemu-devel@nongnu.org; Fri, 10 Jul 2009 15:43:13 -0400 Received: from exim by lists.gnu.org with spam-scanned (Exim 4.43) id 1MPLzv-0007ta-Kp for qemu-devel@nongnu.org; Fri, 10 Jul 2009 15:43:11 -0400 Received: from [199.232.76.173] (port=40596 helo=monty-python.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1MPLzv-0007tX-Fu for qemu-devel@nongnu.org; Fri, 10 Jul 2009 15:43:07 -0400 Received: from mx20.gnu.org ([199.232.41.8]:42601) by monty-python.gnu.org with esmtps (TLS-1.0:RSA_AES_256_CBC_SHA1:32) (Exim 4.60) (envelope-from ) id 1MPLzu-0005aE-TV for qemu-devel@nongnu.org; Fri, 10 Jul 2009 15:43:07 -0400 Received: from mail.codesourcery.com ([65.74.133.4]) by mx20.gnu.org with esmtp (Exim 4.60) (envelope-from ) id 1MPLzu-0002Ow-0u for qemu-devel@nongnu.org; Fri, 10 Jul 2009 15:43:06 -0400 From: Paul Brook Subject: Re: [Qemu-devel] [PATCH 01/13] qdev: rework device properties. Date: Fri, 10 Jul 2009 20:42:59 +0100 References: <1247225179-5495-1-git-send-email-kraxel@redhat.com> <200907101813.37430.paul@codesourcery.com> <4A57967A.50008@redhat.com> In-Reply-To: <4A57967A.50008@redhat.com> MIME-Version: 1.0 Content-Type: Text/Plain; charset="iso-8859-15" Content-Transfer-Encoding: 7bit Content-Disposition: inline Message-Id: <200907102043.01584.paul@codesourcery.com> List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Gerd Hoffmann Cc: qemu-devel@nongnu.org On Friday 10 July 2009, Gerd Hoffmann wrote: > On 07/10/09 19:13, Paul Brook wrote: > >> +extern PropertyInfo qdev_prop_uint32; > >> +extern PropertyInfo qdev_prop_hex32; > > > > Having both of these seems wrong. > > Why? > > There are properties which tend to be specified as decimal numbers > (counts, sizes, ...) and some which tend to be specified in hex > (adresses, ioports, ...). I think it is useful to have separate > parse/print functions for them, although they both end up being an > uint32_t. I think this is the wrong distinction. Whether you specify something in hex or decimal (or even binary/octal) is personal user preference. Distinguishing between integers and addresses may make sense, as those actually have different types. > >> + .name = "fifo-size", > >> + .info =&qdev_prop_uint32, > >> + .offset = offsetof(SyborgPointerState, fifo_size), > >> + .defval = (uint32_t[]) { 16 }, > > > > This feels kinda crufty. Very easy to get the wrong types. > > I think I can make defval a string instead, then go through > PropertyInfo->parse(). OK? I don't think that's really any better. I guess it may just be something we have to put up with. Possibly have a helper macro that sets all the fields. Something like: .properties = (Property[]) { DEFINE_PROPERTY_UINT32("fifo-size", SyborgPointerState, fifo_size, 16) } Ideally we'd have a single DEFINE_PROPERTY macro and the type would be figured out from typeof(fifo_size), but I can't think how to do that. > >> +int qdev_prop_set_uint32(DeviceState *dev, const char *name, uint32_t > >> value); > > > > Why does this return a value? > > It can fail if the size check (soon to be type check) mentioned above > failed. Such a failure would be a clear qemu bug though, so maybe > abort() instead? Yes. Returning a status code then never checking it is completely pointless. Paul