From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:47151) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1W8tMd-0002LX-Ux for qemu-devel@nongnu.org; Thu, 30 Jan 2014 10:17:16 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1W8tMY-0006rC-Vm for qemu-devel@nongnu.org; Thu, 30 Jan 2014 10:17:11 -0500 Received: from mx1.redhat.com ([209.132.183.28]:20804) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1W8tMY-0006r5-Mj for qemu-devel@nongnu.org; Thu, 30 Jan 2014 10:17:06 -0500 Message-ID: <52EA6CF0.4070901@redhat.com> Date: Thu, 30 Jan 2014 08:17:04 -0700 From: Eric Blake MIME-Version: 1.0 References: <1391087394-17914-1-git-send-email-pbonzini@redhat.com> <1391087394-17914-11-git-send-email-pbonzini@redhat.com> In-Reply-To: <1391087394-17914-11-git-send-email-pbonzini@redhat.com> Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="9tf1EUx6ManGPTHGTWKwQsj0nXg83sQ07" Subject: Re: [Qemu-devel] [PATCH 10/12] qdev: remove hex8/32/64 property types List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Paolo Bonzini , qemu-devel@nongnu.org Cc: armbru@redhat.com, afaerber@suse.de This is an OpenPGP/MIME signed message (RFC 4880 and 3156) --9tf1EUx6ManGPTHGTWKwQsj0nXg83sQ07 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable On 01/30/2014 06:09 AM, Paolo Bonzini wrote: > Replace them with uint8/32/64. >=20 > Signed-off-by: Paolo Bonzini > --- Reviewed-by: Eric Blake > +++ b/hw/audio/pcspk.c > @@ -181,7 +181,7 @@ static void pcspk_realizefn(DeviceState *dev, Error= **errp) > } > =20 > static Property pcspk_properties[] =3D { > - DEFINE_PROP_HEX32("iobase", PCSpkState, iobase, -1), > + DEFINE_PROP_UINT32("iobase", PCSpkState, iobase, -1), When there's multiple properties, I can see the value of alignment. But for this instance, the two spaces before -1 seem spurious; you could fix them in this patch. > +++ b/hw/char/parallel.c > @@ -595,7 +595,7 @@ bool parallel_mm_init(MemoryRegion *address_space, > =20 > static Property parallel_isa_properties[] =3D { > DEFINE_PROP_UINT32("index", ISAParallelState, index, -1), > - DEFINE_PROP_HEX32("iobase", ISAParallelState, iobase, -1), > + DEFINE_PROP_UINT32("iobase", ISAParallelState, iobase, -1), > DEFINE_PROP_UINT32("irq", ISAParallelState, isairq, 7), > DEFINE_PROP_CHR("chardev", ISAParallelState, state.chr), And alignment looks all messed up on this one. > DEFINE_PROP_END_OF_LIST(), > diff --git a/hw/char/serial-isa.c b/hw/char/serial-isa.c > index 5cb77b3..c9fcb27 100644 > --- a/hw/char/serial-isa.c > +++ b/hw/char/serial-isa.c > @@ -88,7 +88,7 @@ static const VMStateDescription vmstate_isa_serial =3D= { > =20 > static Property serial_isa_properties[] =3D { > DEFINE_PROP_UINT32("index", ISASerialState, index, -1), > - DEFINE_PROP_HEX32("iobase", ISASerialState, iobase, -1), > + DEFINE_PROP_UINT32("iobase", ISASerialState, iobase, -1), Drop a space before ISASerialState to make this one look nice. > +++ b/hw/display/tcx.c > @@ -617,11 +617,11 @@ static int tcx_init1(SysBusDevice *dev) > } > =20 > static Property tcx_properties[] =3D { > - DEFINE_PROP_HEX32("vram_size", TCXState, vram_size, -1), > + DEFINE_PROP_UINT32("vram_size", TCXState, vram_size, -1), > DEFINE_PROP_UINT16("width", TCXState, width, -1), > DEFINE_PROP_UINT16("height", TCXState, height, -1), > DEFINE_PROP_UINT16("depth", TCXState, depth, -1), > - DEFINE_PROP_HEX64("prom_addr", TCXState, prom_addr, -1), > + DEFINE_PROP_UINT64("prom_addr", TCXState, prom_addr, -1), Another one where alignment is now funky. > +++ b/hw/ide/isa.c > @@ -104,8 +104,8 @@ ISADevice *isa_ide_init(ISABus *bus, int iobase, in= t iobase2, int isairq, > } > =20 > static Property isa_ide_properties[] =3D { > - DEFINE_PROP_HEX32("iobase", ISAIDEState, iobase, 0x1f0), > - DEFINE_PROP_HEX32("iobase2", ISAIDEState, iobase2, 0x3f6), > + DEFINE_PROP_UINT32("iobase", ISAIDEState, iobase, 0x1f0), > + DEFINE_PROP_UINT32("iobase2", ISAIDEState, iobase2, 0x3f6), > DEFINE_PROP_UINT32("irq", ISAIDEState, isairq, 14), And another one. > DEFINE_PROP_END_OF_LIST(), > }; > diff --git a/hw/ide/qdev.c b/hw/ide/qdev.c > index 18c4b7e..6e475e6 100644 > --- a/hw/ide/qdev.c > +++ b/hw/ide/qdev.c > @@ -206,7 +206,7 @@ static int ide_drive_initfn(IDEDevice *dev) > #define DEFINE_IDE_DEV_PROPERTIES() \ > DEFINE_BLOCK_PROPERTIES(IDEDrive, dev.conf), \ > DEFINE_PROP_STRING("ver", IDEDrive, dev.version), \ > - DEFINE_PROP_HEX64("wwn", IDEDrive, dev.wwn, 0), \ > + DEFINE_PROP_UINT64("wwn", IDEDrive, dev.wwn, 0), \ Here, the trailing \ lost alignment. > +++ b/hw/intc/i8259_common.c > @@ -123,9 +123,9 @@ static const VMStateDescription vmstate_pic_common = =3D { > }; > =20 > static Property pic_properties_common[] =3D { > - DEFINE_PROP_HEX32("iobase", PICCommonState, iobase, -1), > - DEFINE_PROP_HEX32("elcr_addr", PICCommonState, elcr_addr, -1), > - DEFINE_PROP_HEX8("elcr_mask", PICCommonState, elcr_mask, -1), > + DEFINE_PROP_UINT32("iobase", PICCommonState, iobase, -1), > + DEFINE_PROP_UINT32("elcr_addr", PICCommonState, elcr_addr, -1), > + DEFINE_PROP_UINT8("elcr_mask", PICCommonState, elcr_mask, -1), > DEFINE_PROP_BIT("master", PICCommonState, master, 0, false), Here there's no alignment, but lots of doubled spaces. > +++ b/hw/misc/applesmc.c > @@ -250,7 +250,7 @@ static void applesmc_isa_realize(DeviceState *dev, = Error **errp) > } > =20 > static Property applesmc_isa_properties[] =3D { > - DEFINE_PROP_HEX32("iobase", AppleSMCState, iobase, > + DEFINE_PROP_UINT32("iobase", AppleSMCState, iobase, > APPLESMC_DEFAULT_IOBASE), Indentation is now off. > +++ b/hw/net/ne2000-isa.c > @@ -86,7 +86,7 @@ static void isa_ne2000_realizefn(DeviceState *dev, Er= ror **errp) > } > =20 > static Property ne2000_isa_properties[] =3D { > - DEFINE_PROP_HEX32("iobase", ISANE2000State, iobase, 0x300), > + DEFINE_PROP_UINT32("iobase", ISANE2000State, iobase, 0x300), > DEFINE_PROP_UINT32("irq", ISANE2000State, isairq, 9), > DEFINE_NIC_PROPERTIES(ISANE2000State, ne2000.c), Here, the line you didn't touch looks awkward. > +++ b/hw/sd/sdhci.c > @@ -1234,9 +1234,9 @@ const VMStateDescription sdhci_vmstate =3D { > /* Capabilities registers provide information on supported features of= this > * specific host controller implementation */ > static Property sdhci_properties[] =3D { > - DEFINE_PROP_HEX32("capareg", SDHCIState, capareg, > + DEFINE_PROP_UINT32("capareg", SDHCIState, capareg, > SDHC_CAPAB_REG_DEFAULT), Might as well fix indentation while touching this. > +++ b/hw/timer/i8254.c > @@ -342,7 +342,7 @@ static void pit_realizefn(DeviceState *dev, Error *= *err) > } > =20 > static Property pit_properties[] =3D { > - DEFINE_PROP_HEX32("iobase", PITCommonState, iobase, -1), > + DEFINE_PROP_UINT32("iobase", PITCommonState, iobase, -1), > DEFINE_PROP_END_OF_LIST(), Another instance of unneeded double space. > +++ b/hw/usb/host-libusb.c > @@ -1324,8 +1324,8 @@ static Property usb_host_dev_properties[] =3D { > DEFINE_PROP_UINT32("hostbus", USBHostDevice, match.bus_num, 0)= , > DEFINE_PROP_UINT32("hostaddr", USBHostDevice, match.addr, 0)= , > DEFINE_PROP_STRING("hostport", USBHostDevice, match.port), > - DEFINE_PROP_HEX32("vendorid", USBHostDevice, match.vendor_id, 0)= , > - DEFINE_PROP_HEX32("productid", USBHostDevice, match.product_id, 0)= , > + DEFINE_PROP_UINT32("vendorid", USBHostDevice, match.vendor_id, 0= ), > + DEFINE_PROP_UINT32("productid", USBHostDevice, match.product_id, 0= ), > DEFINE_PROP_UINT32("isobufs", USBHostDevice, iso_urb_count, 4)= , Alignment looks off. > +++ b/include/hw/qdev-dma.h > @@ -7,4 +7,4 @@ > * See the COPYING file in the top-level directory. > */ > #define DEFINE_PROP_DMAADDR(_n, _s, _f, _d) = \ > - DEFINE_PROP_HEX64(_n, _s, _f, _d) > + DEFINE_PROP_UINT64(_n, _s, _f, _d) Do we even need this #define, or would it be smarter to just inline the use of DEFINE_PROP_UINT64 in the rest of the file? All my comments are cosmetic, so my review still stands; I'm fine whether you apply this as-is or touch it up per the suggestions. --=20 Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org --9tf1EUx6ManGPTHGTWKwQsj0nXg83sQ07 Content-Type: application/pgp-signature; name="signature.asc" Content-Description: OpenPGP digital signature Content-Disposition: attachment; filename="signature.asc" -----BEGIN PGP SIGNATURE----- Version: GnuPG v1 Comment: Public key at http://people.redhat.com/eblake/eblake.gpg Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/ iQEcBAEBCAAGBQJS6mzwAAoJEKeha0olJ0NqG0AH/iL68HxqBCoZatay2ofaJUk6 Gp82q6nbVr7N3d8CrhCZnf+CGc07oGkEaXXjI+faQBLkQiXMf8Do/YZf8PzR+A8P +f6BDbUCZPxJ1WlV8VJrw828SDlR6z65hAPd5fOWymjQet7WCqqH6Ib5OiCj4EuF d0fchFPwaDj/kB1Qz1ktKopDT2sNsVN8KFclC+m06dRA/tA8DTuib7ATEj0ugnfF ukAufrSC8GA/R02uJRwpXNG9HWsUnYHw4QLk5dqptu1Ma9YjT3fgJ0YqZ18KCCOa dy6+GzHyErXxUR4ak5eNDoo8XxRvTYTFXvGf6W8cvUfxGTIWR6t9EiIc03L7OJY= =p4mW -----END PGP SIGNATURE----- --9tf1EUx6ManGPTHGTWKwQsj0nXg83sQ07--