qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [Qemu-devel] [PATCH 0/7] Clean up use of qdev_init()
@ 2009-10-06 23:15 Markus Armbruster
  2009-10-06 23:15 ` [Qemu-devel] [PATCH 1/7] Unbreak USB autoconnect filters Markus Armbruster
                   ` (6 more replies)
  0 siblings, 7 replies; 8+ messages in thread
From: Markus Armbruster @ 2009-10-06 23:15 UTC (permalink / raw)
  To: qemu-devel

qdev_init() can fail since commit 81a322d4.  Most callers don't bother
to check.  This is less serious than it sounds, because they typically
work with devices for which qdev_init() never fails.  It's still
unclean.

The second to last patch makes the compiler warn when the value of
qdev_init() isn't used.  If that warning triggers after merging this
series, more unchecked calls have crept in since the base of this
series (commit 2637c754).  Feel free to ask me for a respin then.

Changes since v1:

* Drop cleanup of xilinx.h

* Fix a bug that crept into usb-linux since v1

* Cover new uses of qdev_init()

* Clean up test for qdev_init() failure

Markus Armbruster (7):
  Unbreak USB autoconnect filters
  Make qdev_init() destroy the device on failure
  Check return value of qdev_init()
  New qdev_init_nofail()
  Make isa_create() terminate program on failure
  Warn if value of qdev_init() isn't checked
  Clean up test for qdev_init() failure

 hw/apb_pci.c      |    2 +-
 hw/arm_sysctl.c   |    2 +-
 hw/armv7m.c       |    6 +++---
 hw/axis_dev88.c   |    2 +-
 hw/escc.c         |    4 ++--
 hw/esp.c          |    2 +-
 hw/etraxfs.c      |    2 +-
 hw/fdc.c          |    8 +++-----
 hw/grackle_pci.c  |    2 +-
 hw/i2c.c          |    2 +-
 hw/ide/isa.c      |    2 +-
 hw/ide/pci.c      |    2 +-
 hw/ide/qdev.c     |    2 +-
 hw/integratorcp.c |    2 +-
 hw/isa-bus.c      |   11 ++++-------
 hw/m48t59.c       |    4 ++--
 hw/mc146818rtc.c  |    2 +-
 hw/mips_malta.c   |    2 +-
 hw/musicpal.c     |    4 ++--
 hw/ne2000-isa.c   |    2 +-
 hw/parallel.c     |    2 +-
 hw/pc.c           |    2 +-
 hw/pci-hotplug.c  |    4 ++--
 hw/pci.c          |    5 +++--
 hw/piix_pci.c     |    2 +-
 hw/qdev.c         |   26 ++++++++++++++++++++++----
 hw/qdev.h         |    3 ++-
 hw/scsi-bus.c     |    4 +++-
 hw/serial.c       |    2 +-
 hw/smc91c111.c    |    2 +-
 hw/ssi.c          |    2 +-
 hw/stellaris.c    |    2 +-
 hw/sun4m.c        |   28 ++++++++++++++--------------
 hw/sun4u.c        |    4 ++--
 hw/syborg.c       |    4 ++--
 hw/sysbus.c       |    2 +-
 hw/unin_pci.c     |    2 +-
 hw/usb-bus.c      |    2 +-
 hw/usb-msd.c      |    3 ++-
 hw/vga-pci.c      |    2 +-
 hw/xilinx.h       |    6 +++---
 usb-linux.c       |    8 ++++----
 42 files changed, 100 insertions(+), 82 deletions(-)

^ permalink raw reply	[flat|nested] 8+ messages in thread

* [Qemu-devel] [PATCH 1/7] Unbreak USB autoconnect filters
  2009-10-06 23:15 [Qemu-devel] [PATCH 0/7] Clean up use of qdev_init() Markus Armbruster
@ 2009-10-06 23:15 ` Markus Armbruster
  2009-10-06 23:15 ` [Qemu-devel] [PATCH 2/7] Make qdev_init() destroy the device on failure Markus Armbruster
                   ` (5 subsequent siblings)
  6 siblings, 0 replies; 8+ messages in thread
From: Markus Armbruster @ 2009-10-06 23:15 UTC (permalink / raw)
  To: qemu-devel

Commit 22f84e73 added a qdev_init() missing on the path through
usb_host_device_open(), but that broke the path through
usb_host_auto_scan(), which already had one.  Remove that one.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 usb-linux.c |    2 --
 1 files changed, 0 insertions(+), 2 deletions(-)

diff --git a/usb-linux.c b/usb-linux.c
index 2b7b092..77cbf1b 100644
--- a/usb-linux.c
+++ b/usb-linux.c
@@ -1396,8 +1396,6 @@ static int usb_host_auto_scan(void *opaque, int bus_num, int addr,
         dprintf("husb: auto open: bus_num %d addr %d\n", bus_num, addr);
 
 	dev = usb_host_device_open_addr(bus_num, addr, product_name);
-	if (dev)
-            qdev_init(&dev->qdev);
     }
 
     return 0;
-- 
1.6.2.5

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [Qemu-devel] [PATCH 2/7] Make qdev_init() destroy the device on failure
  2009-10-06 23:15 [Qemu-devel] [PATCH 0/7] Clean up use of qdev_init() Markus Armbruster
  2009-10-06 23:15 ` [Qemu-devel] [PATCH 1/7] Unbreak USB autoconnect filters Markus Armbruster
@ 2009-10-06 23:15 ` Markus Armbruster
  2009-10-06 23:15 ` [Qemu-devel] [PATCH 3/7] Check return value of qdev_init() Markus Armbruster
                   ` (4 subsequent siblings)
  6 siblings, 0 replies; 8+ messages in thread
From: Markus Armbruster @ 2009-10-06 23:15 UTC (permalink / raw)
  To: qemu-devel

Before, every caller had to do this.  Only two actually did.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 hw/isa-bus.c |    1 -
 hw/qdev.c    |    9 ++++++---
 2 files changed, 6 insertions(+), 4 deletions(-)

diff --git a/hw/isa-bus.c b/hw/isa-bus.c
index 4ecc0f8..fb90be4 100644
--- a/hw/isa-bus.c
+++ b/hw/isa-bus.c
@@ -127,7 +127,6 @@ ISADevice *isa_create_simple(const char *name)
 
     dev = isa_create(name);
     if (qdev_init(&dev->qdev) != 0) {
-        qdev_free(&dev->qdev);
         return NULL;
     }
     return dev;
diff --git a/hw/qdev.c b/hw/qdev.c
index 253e255..8a62001 100644
--- a/hw/qdev.c
+++ b/hw/qdev.c
@@ -216,7 +216,6 @@ DeviceState *qdev_device_add(QemuOpts *opts)
     }
     if (qdev_init(qdev) != 0) {
         qemu_error("Error initializing device %s\n", driver);
-        qdev_free(qdev);
         return NULL;
     }
     qdev->opts = opts;
@@ -232,15 +231,19 @@ static void qdev_reset(void *opaque)
 
 /* Initialize a device.  Device properties should be set before calling
    this function.  IRQs and MMIO regions should be connected/mapped after
-   calling this function.  */
+   calling this function.
+   On failure, destroy the device and return negative value.
+   Return 0 on success.  */
 int qdev_init(DeviceState *dev)
 {
     int rc;
 
     assert(dev->state == DEV_STATE_CREATED);
     rc = dev->info->init(dev, dev->info);
-    if (rc < 0)
+    if (rc < 0) {
+        qdev_free(dev);
         return rc;
+    }
     qemu_register_reset(qdev_reset, dev);
     if (dev->info->vmsd)
         vmstate_register(-1, dev->info->vmsd, dev);
-- 
1.6.2.5

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [Qemu-devel] [PATCH 3/7] Check return value of qdev_init()
  2009-10-06 23:15 [Qemu-devel] [PATCH 0/7] Clean up use of qdev_init() Markus Armbruster
  2009-10-06 23:15 ` [Qemu-devel] [PATCH 1/7] Unbreak USB autoconnect filters Markus Armbruster
  2009-10-06 23:15 ` [Qemu-devel] [PATCH 2/7] Make qdev_init() destroy the device on failure Markus Armbruster
@ 2009-10-06 23:15 ` Markus Armbruster
  2009-10-06 23:15 ` [Qemu-devel] [PATCH 4/7] New qdev_init_nofail() Markus Armbruster
                   ` (3 subsequent siblings)
  6 siblings, 0 replies; 8+ messages in thread
From: Markus Armbruster @ 2009-10-06 23:15 UTC (permalink / raw)
  To: qemu-devel

But do so only where it may actually fail.  Leave the rest for the
next commit.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 hw/pci-hotplug.c |    4 ++--
 hw/pci.c         |    1 +
 hw/scsi-bus.c    |    4 +++-
 hw/usb-msd.c     |    3 ++-
 usb-linux.c      |    6 ++++--
 5 files changed, 12 insertions(+), 6 deletions(-)

diff --git a/hw/pci-hotplug.c b/hw/pci-hotplug.c
index c38a127..ccd2cf3 100644
--- a/hw/pci-hotplug.c
+++ b/hw/pci-hotplug.c
@@ -172,8 +172,8 @@ static PCIDevice *qemu_pci_hot_add_storage(Monitor *mon,
     default:
         dev = NULL;
     }
-    if (dev)
-        qdev_init(&dev->qdev);
+    if (!dev || qdev_init(&dev->qdev) < 0)
+        return NULL;
     return dev;
 }
 
diff --git a/hw/pci.c b/hw/pci.c
index 1debdab..da5cc56 100644
--- a/hw/pci.c
+++ b/hw/pci.c
@@ -833,6 +833,7 @@ static const char * const pci_nic_names[] = {
 };
 
 /* Initialize a PCI NIC.  */
+/* FIXME callers should check for failure, but don't */
 PCIDevice *pci_nic_init(NICInfo *nd, const char *default_model,
                         const char *default_devaddr)
 {
diff --git a/hw/scsi-bus.c b/hw/scsi-bus.c
index fe8991e..41992e5 100644
--- a/hw/scsi-bus.c
+++ b/hw/scsi-bus.c
@@ -82,6 +82,7 @@ void scsi_qdev_register(SCSIDeviceInfo *info)
 }
 
 /* handle legacy '-drive if=scsi,...' cmd line args */
+/* FIXME callers should check for failure, but don't */
 SCSIDevice *scsi_bus_legacy_add_drive(SCSIBus *bus, DriveInfo *dinfo, int unit)
 {
     const char *driver;
@@ -91,7 +92,8 @@ SCSIDevice *scsi_bus_legacy_add_drive(SCSIBus *bus, DriveInfo *dinfo, int unit)
     dev = qdev_create(&bus->qbus, driver);
     qdev_prop_set_uint32(dev, "scsi-id", unit);
     qdev_prop_set_drive(dev, "drive", dinfo);
-    qdev_init(dev);
+    if (qdev_init(dev) < 0)
+        return NULL;
     return DO_UPCAST(SCSIDevice, qdev, dev);
 }
 
diff --git a/hw/usb-msd.c b/hw/usb-msd.c
index e090014..dd3010e 100644
--- a/hw/usb-msd.c
+++ b/hw/usb-msd.c
@@ -571,7 +571,8 @@ USBDevice *usb_msd_init(const char *filename)
     /* create guest device */
     dev = usb_create(NULL /* FIXME */, "QEMU USB MSD");
     qdev_prop_set_drive(&dev->qdev, "drive", dinfo);
-    qdev_init(&dev->qdev);
+    if (qdev_init(&dev->qdev) < 0)
+        return NULL;
 
     return dev;
 }
diff --git a/usb-linux.c b/usb-linux.c
index 77cbf1b..9e5d9c4 100644
--- a/usb-linux.c
+++ b/usb-linux.c
@@ -980,12 +980,14 @@ static USBDevice *usb_host_device_open_addr(int bus_num, int addr, const char *p
 
     hostdev_link(dev);
 
-    qdev_init(&d->qdev);
+    if (qdev_init(&d->qdev) < 0)
+        goto fail_no_qdev;
     return (USBDevice *) dev;
 
 fail:
     if (d)
         qdev_free(&d->qdev);
+fail_no_qdev:
     if (fd != -1)
         close(fd);
     return NULL;
@@ -1389,7 +1391,7 @@ static int usb_host_auto_scan(void *opaque, int bus_num, int addr,
 
         /* We got a match */
 
-        /* Allredy attached ? */
+        /* Already attached ? */
         if (hostdev_find(bus_num, addr))
             return 0;
 
-- 
1.6.2.5

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [Qemu-devel] [PATCH 4/7] New qdev_init_nofail()
  2009-10-06 23:15 [Qemu-devel] [PATCH 0/7] Clean up use of qdev_init() Markus Armbruster
                   ` (2 preceding siblings ...)
  2009-10-06 23:15 ` [Qemu-devel] [PATCH 3/7] Check return value of qdev_init() Markus Armbruster
@ 2009-10-06 23:15 ` Markus Armbruster
  2009-10-06 23:15 ` [Qemu-devel] [PATCH 5/7] Make isa_create() terminate program on failure Markus Armbruster
                   ` (2 subsequent siblings)
  6 siblings, 0 replies; 8+ messages in thread
From: Markus Armbruster @ 2009-10-06 23:15 UTC (permalink / raw)
  To: qemu-devel

Like qdev_init(), but terminate program via hw_error() instead of
returning an error value.

Use it instead of qdev_init() where terminating the program on failure
is okay, either because it's during machine construction, or because
we know that failure can't happen.

Because relying in the latter is somewhat unclean, and the former is
not always obvious, it would be nice to go back to qdev_init() in the
not-so-obvious cases, only with proper error handling.  I'm leaving
that for another day, because it involves making sure that error
values are properly checked by all callers.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 hw/apb_pci.c      |    2 +-
 hw/arm_sysctl.c   |    2 +-
 hw/armv7m.c       |    6 +++---
 hw/axis_dev88.c   |    2 +-
 hw/escc.c         |    4 ++--
 hw/esp.c          |    2 +-
 hw/etraxfs.c      |    2 +-
 hw/fdc.c          |    6 ++----
 hw/grackle_pci.c  |    2 +-
 hw/i2c.c          |    2 +-
 hw/ide/pci.c      |    2 +-
 hw/integratorcp.c |    2 +-
 hw/isa-bus.c      |    2 +-
 hw/m48t59.c       |    4 ++--
 hw/mc146818rtc.c  |    2 +-
 hw/mips_malta.c   |    2 +-
 hw/musicpal.c     |    4 ++--
 hw/ne2000-isa.c   |    2 +-
 hw/pc.c           |    2 +-
 hw/pci.c          |    4 ++--
 hw/piix_pci.c     |    2 +-
 hw/qdev.c         |   15 +++++++++++++++
 hw/qdev.h         |    1 +
 hw/smc91c111.c    |    2 +-
 hw/ssi.c          |    2 +-
 hw/stellaris.c    |    2 +-
 hw/sun4m.c        |   28 ++++++++++++++--------------
 hw/sun4u.c        |    4 ++--
 hw/syborg.c       |    4 ++--
 hw/sysbus.c       |    2 +-
 hw/unin_pci.c     |    2 +-
 hw/usb-bus.c      |    2 +-
 hw/vga-pci.c      |    2 +-
 hw/xilinx.h       |    6 +++---
 34 files changed, 72 insertions(+), 58 deletions(-)

diff --git a/hw/apb_pci.c b/hw/apb_pci.c
index eb77042..72f15af 100644
--- a/hw/apb_pci.c
+++ b/hw/apb_pci.c
@@ -236,7 +236,7 @@ PCIBus *pci_apb_init(target_phys_addr_t special_base,
 
     /* Ultrasparc PBM main bus */
     dev = qdev_create(NULL, "pbm");
-    qdev_init(dev);
+    qdev_init_nofail(dev);
     s = sysbus_from_qdev(dev);
     /* apb_config */
     sysbus_mmio_map(s, 0, special_base + 0x2000ULL);
diff --git a/hw/arm_sysctl.c b/hw/arm_sysctl.c
index 26300ef..72c7ccb 100644
--- a/hw/arm_sysctl.c
+++ b/hw/arm_sysctl.c
@@ -212,7 +212,7 @@ void arm_sysctl_init(uint32_t base, uint32_t sys_id)
 
     dev = qdev_create(NULL, "realview_sysctl");
     qdev_prop_set_uint32(dev, "sys_id", sys_id);
-    qdev_init(dev);
+    qdev_init_nofail(dev);
     sysbus_mmio_map(sysbus_from_qdev(dev), 0, base);
 }
 
diff --git a/hw/armv7m.c b/hw/armv7m.c
index a96288d..034323d 100644
--- a/hw/armv7m.c
+++ b/hw/armv7m.c
@@ -141,12 +141,12 @@ static void armv7m_bitband_init(void)
 
     dev = qdev_create(NULL, "ARM,bitband-memory");
     qdev_prop_set_uint32(dev, "base", 0x20000000);
-    qdev_init(dev);
+    qdev_init_nofail(dev);
     sysbus_mmio_map(sysbus_from_qdev(dev), 0, 0x22000000);
 
     dev = qdev_create(NULL, "ARM,bitband-memory");
     qdev_prop_set_uint32(dev, "base", 0x40000000);
-    qdev_init(dev);
+    qdev_init_nofail(dev);
     sysbus_mmio_map(sysbus_from_qdev(dev), 0, 0x42000000);
 }
 
@@ -202,7 +202,7 @@ qemu_irq *armv7m_init(int flash_size, int sram_size,
 
     nvic = qdev_create(NULL, "armv7m_nvic");
     env->v7m.nvic = nvic;
-    qdev_init(nvic);
+    qdev_init_nofail(nvic);
     cpu_pic = arm_pic_init_cpu(env);
     sysbus_connect_irq(sysbus_from_qdev(nvic), 0, cpu_pic[ARM_PIC_CPU_IRQ]);
     for (i = 0; i < 64; i++) {
diff --git a/hw/axis_dev88.c b/hw/axis_dev88.c
index 81a41c9..d6f14bc 100644
--- a/hw/axis_dev88.c
+++ b/hw/axis_dev88.c
@@ -300,7 +300,7 @@ void axisdev88_init (ram_addr_t ram_size,
     dev = qdev_create(NULL, "etraxfs,pic");
     /* FIXME: Is there a proper way to signal vectors to the CPU core?  */
     qdev_prop_set_ptr(dev, "interrupt_vector", &env->interrupt_vector);
-    qdev_init(dev);
+    qdev_init_nofail(dev);
     s = sysbus_from_qdev(dev);
     sysbus_mmio_map(s, 0, 0x3001c000);
     sysbus_connect_irq(s, 0, cpu_irq[0]);
diff --git a/hw/escc.c b/hw/escc.c
index 382719d..2970428 100644
--- a/hw/escc.c
+++ b/hw/escc.c
@@ -741,7 +741,7 @@ int escc_init(target_phys_addr_t base, qemu_irq irqA, qemu_irq irqB,
     qdev_prop_set_chr(dev, "chrA", chrA);
     qdev_prop_set_uint32(dev, "chnBtype", ser);
     qdev_prop_set_uint32(dev, "chnAtype", ser);
-    qdev_init(dev);
+    qdev_init_nofail(dev);
     s = sysbus_from_qdev(dev);
     sysbus_connect_irq(s, 0, irqB);
     sysbus_connect_irq(s, 1, irqA);
@@ -904,7 +904,7 @@ void slavio_serial_ms_kbd_init(target_phys_addr_t base, qemu_irq irq,
     qdev_prop_set_chr(dev, "chrA", NULL);
     qdev_prop_set_uint32(dev, "chnBtype", mouse);
     qdev_prop_set_uint32(dev, "chnAtype", kbd);
-    qdev_init(dev);
+    qdev_init_nofail(dev);
     s = sysbus_from_qdev(dev);
     sysbus_connect_irq(s, 0, irq);
     sysbus_connect_irq(s, 1, irq);
diff --git a/hw/esp.c b/hw/esp.c
index 5d86020..c2567a6 100644
--- a/hw/esp.c
+++ b/hw/esp.c
@@ -647,7 +647,7 @@ void esp_init(target_phys_addr_t espaddr, int it_shift,
     esp->dma_memory_write = dma_memory_write;
     esp->dma_opaque = dma_opaque;
     esp->it_shift = it_shift;
-    qdev_init(dev);
+    qdev_init_nofail(dev);
     s = sysbus_from_qdev(dev);
     sysbus_connect_irq(s, 0, irq);
     sysbus_mmio_map(s, 0, espaddr);
diff --git a/hw/etraxfs.c b/hw/etraxfs.c
index 4f451c5..7937e48 100644
--- a/hw/etraxfs.c
+++ b/hw/etraxfs.c
@@ -92,7 +92,7 @@ void bareetraxfs_init (ram_addr_t ram_size,
     dev = qdev_create(NULL, "etraxfs,pic");
     /* FIXME: Is there a proper way to signal vectors to the CPU core?  */
     qdev_prop_set_ptr(dev, "interrupt_vector", &env->interrupt_vector);
-    qdev_init(dev);
+    qdev_init_nofail(dev);
     s = sysbus_from_qdev(dev);
     sysbus_mmio_map(s, 0, 0x3001c000);
     sysbus_connect_irq(s, 0, cpu_irq[0]);
diff --git a/hw/fdc.c b/hw/fdc.c
index 6a6e7a8..f1b2f9a 100644
--- a/hw/fdc.c
+++ b/hw/fdc.c
@@ -1866,8 +1866,7 @@ fdctrl_t *fdctrl_init_sysbus(qemu_irq irq, int dma_chann,
     fdctrl->dma_chann = dma_chann; /* FIXME */
     qdev_prop_set_drive(dev, "driveA", fds[0]);
     qdev_prop_set_drive(dev, "driveB", fds[1]);
-    if (qdev_init(dev) != 0)
-        return NULL;
+    qdev_init_nofail(dev);
     sysbus_connect_irq(&sys->busdev, 0, irq);
     sysbus_mmio_map(&sys->busdev, 0, mmio_base);
 
@@ -1883,8 +1882,7 @@ fdctrl_t *sun4m_fdctrl_init (qemu_irq irq, target_phys_addr_t io_base,
 
     dev = qdev_create(NULL, "SUNW,fdtwo");
     qdev_prop_set_drive(dev, "drive", fds[0]);
-    if (qdev_init(dev) != 0)
-        return NULL;
+    qdev_init_nofail(dev);
     sys = DO_UPCAST(fdctrl_sysbus_t, busdev.qdev, dev);
     fdctrl = &sys->state;
     sysbus_connect_irq(&sys->busdev, 0, irq);
diff --git a/hw/grackle_pci.c b/hw/grackle_pci.c
index d878cf6..b49cf1e 100644
--- a/hw/grackle_pci.c
+++ b/hw/grackle_pci.c
@@ -138,7 +138,7 @@ PCIBus *pci_grackle_init(uint32_t base, qemu_irq *pic)
     GrackleState *d;
 
     dev = qdev_create(NULL, "grackle");
-    qdev_init(dev);
+    qdev_init_nofail(dev);
     s = sysbus_from_qdev(dev);
     d = FROM_SYSBUS(GrackleState, s);
     d->host_state.bus = pci_register_bus(&d->busdev.qdev, "pci",
diff --git a/hw/i2c.c b/hw/i2c.c
index 6cd4701..5c291ce 100644
--- a/hw/i2c.c
+++ b/hw/i2c.c
@@ -189,6 +189,6 @@ DeviceState *i2c_create_slave(i2c_bus *bus, const char *name, uint8_t addr)
 
     dev = qdev_create(&bus->qbus, name);
     qdev_prop_set_uint8(dev, "address", addr);
-    qdev_init(dev);
+    qdev_init_nofail(dev);
     return dev;
 }
diff --git a/hw/ide/pci.c b/hw/ide/pci.c
index 6d090bc..9504e44 100644
--- a/hw/ide/pci.c
+++ b/hw/ide/pci.c
@@ -445,7 +445,7 @@ void pci_cmd646_ide_init(PCIBus *bus, DriveInfo **hd_table,
 
     dev = pci_create(bus, -1, "CMD646 IDE");
     qdev_prop_set_uint32(&dev->qdev, "secondary", secondary_ide_enabled);
-    qdev_init(&dev->qdev);
+    qdev_init_nofail(&dev->qdev);
 
     pci_ide_create_devs(dev, hd_table);
 }
diff --git a/hw/integratorcp.c b/hw/integratorcp.c
index 21e7712..bee8298 100644
--- a/hw/integratorcp.c
+++ b/hw/integratorcp.c
@@ -477,7 +477,7 @@ static void integratorcp_init(ram_addr_t ram_size,
 
     dev = qdev_create(NULL, "integrator_core");
     qdev_prop_set_uint32(dev, "memsz", ram_size >> 20);
-    qdev_init(dev);
+    qdev_init_nofail(dev);
     sysbus_mmio_map((SysBusDevice *)dev, 0, 0x10000000);
 
     cpu_pic = arm_pic_init_cpu(env);
diff --git a/hw/isa-bus.c b/hw/isa-bus.c
index fb90be4..f7e73d2 100644
--- a/hw/isa-bus.c
+++ b/hw/isa-bus.c
@@ -45,7 +45,7 @@ ISABus *isa_bus_new(DeviceState *dev)
     }
     if (NULL == dev) {
         dev = qdev_create(NULL, "isabus-bridge");
-        qdev_init(dev);
+        qdev_init_nofail(dev);
     }
 
     isabus = FROM_QBUS(ISABus, qbus_create(&isa_bus_info, dev, NULL));
diff --git a/hw/m48t59.c b/hw/m48t59.c
index b9892cc..0f45071 100644
--- a/hw/m48t59.c
+++ b/hw/m48t59.c
@@ -636,7 +636,7 @@ m48t59_t *m48t59_init (qemu_irq IRQ, target_phys_addr_t mem_base,
     qdev_prop_set_uint32(dev, "type", type);
     qdev_prop_set_uint32(dev, "size", size);
     qdev_prop_set_uint32(dev, "io_base", io_base);
-    qdev_init(dev);
+    qdev_init_nofail(dev);
     s = sysbus_from_qdev(dev);
     sysbus_connect_irq(s, 0, IRQ);
     if (io_base != 0) {
@@ -662,7 +662,7 @@ m48t59_t *m48t59_init_isa(uint32_t io_base, uint16_t size, int type)
     qdev_prop_set_uint32(&dev->qdev, "type", type);
     qdev_prop_set_uint32(&dev->qdev, "size", size);
     qdev_prop_set_uint32(&dev->qdev, "io_base", io_base);
-    qdev_init(&dev->qdev);
+    qdev_init_nofail(&dev->qdev);
     d = DO_UPCAST(M48t59ISAState, busdev, dev);
     s = &d->state;
 
diff --git a/hw/mc146818rtc.c b/hw/mc146818rtc.c
index bec5687..d82131a 100644
--- a/hw/mc146818rtc.c
+++ b/hw/mc146818rtc.c
@@ -637,7 +637,7 @@ RTCState *rtc_init(int base_year)
 
     dev = isa_create("mc146818rtc");
     qdev_prop_set_int32(&dev->qdev, "base_year", base_year);
-    qdev_init(&dev->qdev);
+    qdev_init_nofail(&dev->qdev);
     return DO_UPCAST(RTCState, dev, dev);
 }
 
diff --git a/hw/mips_malta.c b/hw/mips_malta.c
index d0266d5..b26bbee 100644
--- a/hw/mips_malta.c
+++ b/hw/mips_malta.c
@@ -933,7 +933,7 @@ void mips_malta_init (ram_addr_t ram_size,
         eeprom = qdev_create((BusState *)smbus, "smbus-eeprom");
         qdev_prop_set_uint8(eeprom, "address", 0x50 + i);
         qdev_prop_set_ptr(eeprom, "data", eeprom_buf + (i * 256));
-        qdev_init(eeprom);
+        qdev_init_nofail(eeprom);
     }
     pit = pit_init(0x40, isa_reserve_irq(0));
     DMA_init(0);
diff --git a/hw/musicpal.c b/hw/musicpal.c
index 1fad36f..02d4c70 100644
--- a/hw/musicpal.c
+++ b/hw/musicpal.c
@@ -1551,7 +1551,7 @@ static void musicpal_init(ram_addr_t ram_size,
     qemu_check_nic_model(&nd_table[0], "mv88w8618");
     dev = qdev_create(NULL, "mv88w8618_eth");
     dev->nd = &nd_table[0];
-    qdev_init(dev);
+    qdev_init_nofail(dev);
     sysbus_mmio_map(sysbus_from_qdev(dev), 0, MP_ETH_BASE);
     sysbus_connect_irq(sysbus_from_qdev(dev), 0, pic[MP_ETH_IRQ]);
 
@@ -1589,7 +1589,7 @@ static void musicpal_init(ram_addr_t ram_size,
     dev = qdev_create(NULL, "mv88w8618_audio");
     s = sysbus_from_qdev(dev);
     qdev_prop_set_ptr(dev, "wm8750", wm8750_dev);
-    qdev_init(dev);
+    qdev_init_nofail(dev);
     sysbus_mmio_map(s, 0, MP_AUDIO_BASE);
     sysbus_connect_irq(s, 0, pic[MP_AUDIO_IRQ]);
 #endif
diff --git a/hw/ne2000-isa.c b/hw/ne2000-isa.c
index 54c0478..e346731 100644
--- a/hw/ne2000-isa.c
+++ b/hw/ne2000-isa.c
@@ -87,7 +87,7 @@ void isa_ne2000_init(int base, int irq, NICInfo *nd)
     dev->qdev.nd = nd; /* hack alert */
     qdev_prop_set_uint32(&dev->qdev, "iobase", base);
     qdev_prop_set_uint32(&dev->qdev, "irq",    irq);
-    qdev_init(&dev->qdev);
+    qdev_init_nofail(&dev->qdev);
 }
 
 static ISADeviceInfo ne2000_isa_info = {
diff --git a/hw/pc.c b/hw/pc.c
index 2ca15a3..19bef49 100644
--- a/hw/pc.c
+++ b/hw/pc.c
@@ -1294,7 +1294,7 @@ static void pc_init1(ram_addr_t ram_size,
             eeprom = qdev_create((BusState *)smbus, "smbus-eeprom");
             qdev_prop_set_uint8(eeprom, "address", 0x50 + i);
             qdev_prop_set_ptr(eeprom, "data", eeprom_buf + (i * 256));
-            qdev_init(eeprom);
+            qdev_init_nofail(eeprom);
         }
         piix4_acpi_system_hot_add_init(pci_bus);
     }
diff --git a/hw/pci.c b/hw/pci.c
index da5cc56..49651d0 100644
--- a/hw/pci.c
+++ b/hw/pci.c
@@ -946,7 +946,7 @@ PCIBus *pci_bridge_init(PCIBus *bus, int devfn, uint16_t vid, uint16_t did,
     dev = pci_create(bus, devfn, "pci-bridge");
     qdev_prop_set_uint32(&dev->qdev, "vendorid", vid);
     qdev_prop_set_uint32(&dev->qdev, "deviceid", did);
-    qdev_init(&dev->qdev);
+    qdev_init_nofail(&dev->qdev);
 
     s = DO_UPCAST(PCIBridge, dev, dev);
     pci_register_secondary_bus(&s->bus, &s->dev, map_irq, name);
@@ -1010,7 +1010,7 @@ PCIDevice *pci_create(PCIBus *bus, int devfn, const char *name)
 PCIDevice *pci_create_simple(PCIBus *bus, int devfn, const char *name)
 {
     PCIDevice *dev = pci_create(bus, devfn, name);
-    qdev_init(&dev->qdev);
+    qdev_init_nofail(&dev->qdev);
     return dev;
 }
 
diff --git a/hw/piix_pci.c b/hw/piix_pci.c
index 3cc7333..ed036fe 100644
--- a/hw/piix_pci.c
+++ b/hw/piix_pci.c
@@ -234,7 +234,7 @@ PCIBus *i440fx_init(PCII440FXState **pi440fx_state, int *piix3_devfn, qemu_irq *
     s = FROM_SYSBUS(I440FXState, sysbus_from_qdev(dev));
     b = pci_bus_new(&s->busdev.qdev, NULL, 0);
     s->bus = b;
-    qdev_init(dev);
+    qdev_init_nofail(dev);
 
     d = pci_create_simple(b, 0, "i440FX");
     *pi440fx_state = DO_UPCAST(PCII440FXState, dev, d);
diff --git a/hw/qdev.c b/hw/qdev.c
index 8a62001..ca60923 100644
--- a/hw/qdev.c
+++ b/hw/qdev.c
@@ -269,6 +269,21 @@ int qdev_simple_unplug_cb(DeviceState *dev)
     return 0;
 }
 
+/* Like qdev_init(), but terminate program via hw_error() instead of
+   returning an error value.  This is okay during machine creation.
+   Don't use for hotplug, because there callers need to recover from
+   failure.  Exception: if you know the device's init() callback can't
+   fail, then qdev_init_nofail() can't fail either, and is therefore
+   usable even then.  But relying on the device implementation that
+   way is somewhat unclean, and best avoided.  */
+void qdev_init_nofail(DeviceState *dev)
+{
+    DeviceInfo *info = dev->info;
+
+    if (qdev_init(dev) < 0)
+        hw_error("Initialization of device %s failed\n", info->name);
+}
+
 /* Unlink device from bus and free the structure.  */
 void qdev_free(DeviceState *dev)
 {
diff --git a/hw/qdev.h b/hw/qdev.h
index 893ae92..b385b25 100644
--- a/hw/qdev.h
+++ b/hw/qdev.h
@@ -101,6 +101,7 @@ struct CompatProperty {
 DeviceState *qdev_create(BusState *bus, const char *name);
 DeviceState *qdev_device_add(QemuOpts *opts);
 int qdev_init(DeviceState *dev);
+void qdev_init_nofail(DeviceState *dev);
 int qdev_unplug(DeviceState *dev);
 void qdev_free(DeviceState *dev);
 int qdev_simple_unplug_cb(DeviceState *dev);
diff --git a/hw/smc91c111.c b/hw/smc91c111.c
index a08bdb0..d58821a 100644
--- a/hw/smc91c111.c
+++ b/hw/smc91c111.c
@@ -735,7 +735,7 @@ void smc91c111_init(NICInfo *nd, uint32_t base, qemu_irq irq)
     qemu_check_nic_model(nd, "smc91c111");
     dev = qdev_create(NULL, "smc91c111");
     dev->nd = nd;
-    qdev_init(dev);
+    qdev_init_nofail(dev);
     s = sysbus_from_qdev(dev);
     sysbus_mmio_map(s, 0, base);
     sysbus_connect_irq(s, 0, irq);
diff --git a/hw/ssi.c b/hw/ssi.c
index 73cb541..cfe7c07 100644
--- a/hw/ssi.c
+++ b/hw/ssi.c
@@ -46,7 +46,7 @@ DeviceState *ssi_create_slave(SSIBus *bus, const char *name)
 {
     DeviceState *dev;
     dev = qdev_create(&bus->qbus, name);
-    qdev_init(dev);
+    qdev_init_nofail(dev);
     return dev;
 }
 
diff --git a/hw/stellaris.c b/hw/stellaris.c
index bcde0a2..1628914 100644
--- a/hw/stellaris.c
+++ b/hw/stellaris.c
@@ -1384,7 +1384,7 @@ static void stellaris_init(const char *kernel_filename, const char *cpu_model,
 
         enet = qdev_create(NULL, "stellaris_enet");
         enet->nd = &nd_table[0];
-        qdev_init(enet);
+        qdev_init_nofail(enet);
         sysbus_mmio_map(sysbus_from_qdev(enet), 0, 0x40048000);
         sysbus_connect_irq(sysbus_from_qdev(enet), 0, pic[42]);
     }
diff --git a/hw/sun4m.c b/hw/sun4m.c
index 01c7cb4..8f46a0f 100644
--- a/hw/sun4m.c
+++ b/hw/sun4m.c
@@ -359,7 +359,7 @@ static void *iommu_init(target_phys_addr_t addr, uint32_t version, qemu_irq irq)
 
     dev = qdev_create(NULL, "iommu");
     qdev_prop_set_uint32(dev, "version", version);
-    qdev_init(dev);
+    qdev_init_nofail(dev);
     s = sysbus_from_qdev(dev);
     sysbus_connect_irq(s, 0, irq);
     sysbus_mmio_map(s, 0, addr);
@@ -375,7 +375,7 @@ static void *sparc32_dma_init(target_phys_addr_t daddr, qemu_irq parent_irq,
 
     dev = qdev_create(NULL, "sparc32_dma");
     qdev_prop_set_ptr(dev, "iommu_opaque", iommu);
-    qdev_init(dev);
+    qdev_init_nofail(dev);
     s = sysbus_from_qdev(dev);
     sysbus_connect_irq(s, 0, parent_irq);
     *dev_irq = qdev_get_gpio_in(dev, 0);
@@ -396,7 +396,7 @@ static void lance_init(NICInfo *nd, target_phys_addr_t leaddr,
     dev = qdev_create(NULL, "lance");
     dev->nd = nd;
     qdev_prop_set_ptr(dev, "dma", dma_opaque);
-    qdev_init(dev);
+    qdev_init_nofail(dev);
     s = sysbus_from_qdev(dev);
     sysbus_mmio_map(s, 0, leaddr);
     sysbus_connect_irq(s, 0, irq);
@@ -413,7 +413,7 @@ static DeviceState *slavio_intctl_init(target_phys_addr_t addr,
     unsigned int i, j;
 
     dev = qdev_create(NULL, "slavio_intctl");
-    qdev_init(dev);
+    qdev_init_nofail(dev);
 
     s = sysbus_from_qdev(dev);
 
@@ -442,7 +442,7 @@ static void slavio_timer_init_all(target_phys_addr_t addr, qemu_irq master_irq,
 
     dev = qdev_create(NULL, "slavio_timer");
     qdev_prop_set_uint32(dev, "num_cpus", num_cpus);
-    qdev_init(dev);
+    qdev_init_nofail(dev);
     s = sysbus_from_qdev(dev);
     sysbus_connect_irq(s, 0, master_irq);
     sysbus_mmio_map(s, 0, addr + SYS_TIMER_OFFSET);
@@ -468,7 +468,7 @@ static void slavio_misc_init(target_phys_addr_t base,
     SysBusDevice *s;
 
     dev = qdev_create(NULL, "slavio_misc");
-    qdev_init(dev);
+    qdev_init_nofail(dev);
     s = sysbus_from_qdev(dev);
     if (base) {
         /* 8 bit registers */
@@ -505,7 +505,7 @@ static void ecc_init(target_phys_addr_t base, qemu_irq irq, uint32_t version)
 
     dev = qdev_create(NULL, "eccmemctl");
     qdev_prop_set_uint32(dev, "version", version);
-    qdev_init(dev);
+    qdev_init_nofail(dev);
     s = sysbus_from_qdev(dev);
     sysbus_connect_irq(s, 0, irq);
     sysbus_mmio_map(s, 0, base);
@@ -520,7 +520,7 @@ static void apc_init(target_phys_addr_t power_base, qemu_irq cpu_halt)
     SysBusDevice *s;
 
     dev = qdev_create(NULL, "apc");
-    qdev_init(dev);
+    qdev_init_nofail(dev);
     s = sysbus_from_qdev(dev);
     /* Power management (APC) XXX: not a Slavio device */
     sysbus_mmio_map(s, 0, power_base);
@@ -539,7 +539,7 @@ static void tcx_init(target_phys_addr_t addr, int vram_size, int width,
     qdev_prop_set_uint16(dev, "width", width);
     qdev_prop_set_uint16(dev, "height", height);
     qdev_prop_set_uint16(dev, "depth", depth);
-    qdev_init(dev);
+    qdev_init_nofail(dev);
     s = sysbus_from_qdev(dev);
     /* 8-bit plane */
     sysbus_mmio_map(s, 0, addr + 0x00800000ULL);
@@ -569,7 +569,7 @@ static void idreg_init(target_phys_addr_t addr)
     SysBusDevice *s;
 
     dev = qdev_create(NULL, "macio_idreg");
-    qdev_init(dev);
+    qdev_init_nofail(dev);
     s = sysbus_from_qdev(dev);
 
     sysbus_mmio_map(s, 0, addr);
@@ -607,7 +607,7 @@ static void prom_init(target_phys_addr_t addr, const char *bios_name)
     int ret;
 
     dev = qdev_create(NULL, "openprom");
-    qdev_init(dev);
+    qdev_init_nofail(dev);
     s = sysbus_from_qdev(dev);
 
     sysbus_mmio_map(s, 0, addr);
@@ -697,7 +697,7 @@ static void ram_init(target_phys_addr_t addr, ram_addr_t RAM_size,
 
     d = FROM_SYSBUS(RamDevice, s);
     d->size = RAM_size;
-    qdev_init(dev);
+    qdev_init_nofail(dev);
 
     sysbus_mmio_map(s, 0, addr);
 }
@@ -1344,7 +1344,7 @@ static DeviceState *sbi_init(target_phys_addr_t addr, qemu_irq **parent_irq)
     unsigned int i;
 
     dev = qdev_create(NULL, "sbi");
-    qdev_init(dev);
+    qdev_init_nofail(dev);
 
     s = sysbus_from_qdev(dev);
 
@@ -1534,7 +1534,7 @@ static DeviceState *sun4c_intctl_init(target_phys_addr_t addr,
     unsigned int i;
 
     dev = qdev_create(NULL, "sun4c_intctl");
-    qdev_init(dev);
+    qdev_init_nofail(dev);
 
     s = sysbus_from_qdev(dev);
 
diff --git a/hw/sun4u.c b/hw/sun4u.c
index 58d708a..276b17b 100644
--- a/hw/sun4u.c
+++ b/hw/sun4u.c
@@ -415,7 +415,7 @@ static void prom_init(target_phys_addr_t addr, const char *bios_name)
     int ret;
 
     dev = qdev_create(NULL, "openprom");
-    qdev_init(dev);
+    qdev_init_nofail(dev);
     s = sysbus_from_qdev(dev);
 
     sysbus_mmio_map(s, 0, addr);
@@ -498,7 +498,7 @@ static void ram_init(target_phys_addr_t addr, ram_addr_t RAM_size)
 
     d = FROM_SYSBUS(RamDevice, s);
     d->size = RAM_size;
-    qdev_init(dev);
+    qdev_init_nofail(dev);
 
     sysbus_mmio_map(s, 0, addr);
 }
diff --git a/hw/syborg.c b/hw/syborg.c
index d8d38d4..2aec769 100644
--- a/hw/syborg.c
+++ b/hw/syborg.c
@@ -65,7 +65,7 @@ static void syborg_init(ram_addr_t ram_size,
 
     dev = qdev_create(NULL, "syborg,timer");
     qdev_prop_set_uint32(dev, "frequency", 1000000);
-    qdev_init(dev);
+    qdev_init_nofail(dev);
     sysbus_mmio_map(sysbus_from_qdev(dev), 0, 0xC0002000);
     sysbus_connect_irq(sysbus_from_qdev(dev), 0, pic[1]);
 
@@ -84,7 +84,7 @@ static void syborg_init(ram_addr_t ram_size,
         qemu_check_nic_model(&nd_table[0], "virtio");
         dev = qdev_create(NULL, "syborg,virtio-net");
         dev->nd = &nd_table[0];
-        qdev_init(dev);
+        qdev_init_nofail(dev);
         s = sysbus_from_qdev(dev);
         sysbus_mmio_map(s, 0, 0xc000c000);
         sysbus_connect_irq(s, 0, pic[9]);
diff --git a/hw/sysbus.c b/hw/sysbus.c
index f6516fd..1f7f138 100644
--- a/hw/sysbus.c
+++ b/hw/sysbus.c
@@ -143,7 +143,7 @@ DeviceState *sysbus_create_varargs(const char *name,
 
     dev = qdev_create(NULL, name);
     s = sysbus_from_qdev(dev);
-    qdev_init(dev);
+    qdev_init_nofail(dev);
     if (addr != (target_phys_addr_t)-1) {
         sysbus_mmio_map(s, 0, addr);
     }
diff --git a/hw/unin_pci.c b/hw/unin_pci.c
index a202153..4abb5c8 100644
--- a/hw/unin_pci.c
+++ b/hw/unin_pci.c
@@ -253,7 +253,7 @@ PCIBus *pci_pmac_init(qemu_irq *pic)
     /* Use values found on a real PowerMac */
     /* Uninorth main bus */
     dev = qdev_create(NULL, "Uni-north main");
-    qdev_init(dev);
+    qdev_init_nofail(dev);
     s = sysbus_from_qdev(dev);
     d = FROM_SYSBUS(UNINState, s);
     d->host_state.bus = pci_register_bus(&d->busdev.qdev, "pci",
diff --git a/hw/usb-bus.c b/hw/usb-bus.c
index 0c63279..98987a1 100644
--- a/hw/usb-bus.c
+++ b/hw/usb-bus.c
@@ -101,7 +101,7 @@ USBDevice *usb_create(USBBus *bus, const char *name)
 USBDevice *usb_create_simple(USBBus *bus, const char *name)
 {
     USBDevice *dev = usb_create(bus, name);
-    qdev_init(&dev->qdev);
+    qdev_init_nofail(&dev->qdev);
     return dev;
 }
 
diff --git a/hw/vga-pci.c b/hw/vga-pci.c
index 7cb2d6f..5e75938 100644
--- a/hw/vga-pci.c
+++ b/hw/vga-pci.c
@@ -128,7 +128,7 @@ int pci_vga_init(PCIBus *bus,
     dev = pci_create(bus, -1, "VGA");
     qdev_prop_set_uint32(&dev->qdev, "bios-offset", vga_bios_offset);
     qdev_prop_set_uint32(&dev->qdev, "bios-size", vga_bios_offset);
-    qdev_init(&dev->qdev);
+    qdev_init_nofail(&dev->qdev);
 
     return 0;
 }
diff --git a/hw/xilinx.h b/hw/xilinx.h
index 070679c..5e6aeea 100644
--- a/hw/xilinx.h
+++ b/hw/xilinx.h
@@ -9,7 +9,7 @@ xilinx_intc_create(target_phys_addr_t base, qemu_irq irq, int kind_of_intr)
 
     dev = qdev_create(NULL, "xilinx,intc");
     qdev_prop_set_uint32(dev, "kind-of-intr", kind_of_intr);
-    qdev_init(dev);
+    qdev_init_nofail(dev);
     sysbus_mmio_map(sysbus_from_qdev(dev), 0, base);
     sysbus_connect_irq(sysbus_from_qdev(dev), 0, irq);
     return dev;
@@ -24,7 +24,7 @@ xilinx_timer_create(target_phys_addr_t base, qemu_irq irq, int nr, int freq)
     dev = qdev_create(NULL, "xilinx,timer");
     qdev_prop_set_uint32(dev, "nr-timers", nr);
     qdev_prop_set_uint32(dev, "frequency", freq);
-    qdev_init(dev);
+    qdev_init_nofail(dev);
     sysbus_mmio_map(sysbus_from_qdev(dev), 0, base);
     sysbus_connect_irq(sysbus_from_qdev(dev), 0, irq);
     return dev;
@@ -43,7 +43,7 @@ xilinx_ethlite_create(NICInfo *nd, target_phys_addr_t base, qemu_irq irq,
     dev->nd = nd;
     qdev_prop_set_uint32(dev, "txpingpong", txpingpong);
     qdev_prop_set_uint32(dev, "rxpingpong", rxpingpong);
-    qdev_init(dev);
+    qdev_init_nofail(dev);
     sysbus_mmio_map(sysbus_from_qdev(dev), 0, base);
     sysbus_connect_irq(sysbus_from_qdev(dev), 0, irq);
     return dev;
-- 
1.6.2.5

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [Qemu-devel] [PATCH 5/7] Make isa_create() terminate program on failure
  2009-10-06 23:15 [Qemu-devel] [PATCH 0/7] Clean up use of qdev_init() Markus Armbruster
                   ` (3 preceding siblings ...)
  2009-10-06 23:15 ` [Qemu-devel] [PATCH 4/7] New qdev_init_nofail() Markus Armbruster
@ 2009-10-06 23:15 ` Markus Armbruster
  2009-10-06 23:16 ` [Qemu-devel] [PATCH 6/7] Warn if value of qdev_init() isn't checked Markus Armbruster
  2009-10-06 23:16 ` [Qemu-devel] [PATCH 7/7] Clean up test for qdev_init() failure Markus Armbruster
  6 siblings, 0 replies; 8+ messages in thread
From: Markus Armbruster @ 2009-10-06 23:15 UTC (permalink / raw)
  To: qemu-devel

Callers don't check the return value anyway.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 hw/isa-bus.c |    8 +++-----
 1 files changed, 3 insertions(+), 5 deletions(-)

diff --git a/hw/isa-bus.c b/hw/isa-bus.c
index f7e73d2..4d489d2 100644
--- a/hw/isa-bus.c
+++ b/hw/isa-bus.c
@@ -114,8 +114,8 @@ ISADevice *isa_create(const char *name)
     DeviceState *dev;
 
     if (!isabus) {
-        fprintf(stderr, "Tried to create isa device %s with no isa bus present.\n", name);
-        return NULL;
+        hw_error("Tried to create isa device %s with no isa bus present.\n",
+                 name);
     }
     dev = qdev_create(&isabus->qbus, name);
     return DO_UPCAST(ISADevice, qdev, dev);
@@ -126,9 +126,7 @@ ISADevice *isa_create_simple(const char *name)
     ISADevice *dev;
 
     dev = isa_create(name);
-    if (qdev_init(&dev->qdev) != 0) {
-        return NULL;
-    }
+    qdev_init_nofail(&dev->qdev);
     return dev;
 }
 
-- 
1.6.2.5

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [Qemu-devel] [PATCH 6/7] Warn if value of qdev_init() isn't checked
  2009-10-06 23:15 [Qemu-devel] [PATCH 0/7] Clean up use of qdev_init() Markus Armbruster
                   ` (4 preceding siblings ...)
  2009-10-06 23:15 ` [Qemu-devel] [PATCH 5/7] Make isa_create() terminate program on failure Markus Armbruster
@ 2009-10-06 23:16 ` Markus Armbruster
  2009-10-06 23:16 ` [Qemu-devel] [PATCH 7/7] Clean up test for qdev_init() failure Markus Armbruster
  6 siblings, 0 replies; 8+ messages in thread
From: Markus Armbruster @ 2009-10-06 23:16 UTC (permalink / raw)
  To: qemu-devel

After qdev_init() fails, the device is gone.  Failure to check runs a
high risk of use-after-free.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 hw/qdev.h |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/hw/qdev.h b/hw/qdev.h
index b385b25..8cd843e 100644
--- a/hw/qdev.h
+++ b/hw/qdev.h
@@ -100,7 +100,7 @@ struct CompatProperty {
 
 DeviceState *qdev_create(BusState *bus, const char *name);
 DeviceState *qdev_device_add(QemuOpts *opts);
-int qdev_init(DeviceState *dev);
+int qdev_init(DeviceState *dev) __attribute__((warn_unused_result));
 void qdev_init_nofail(DeviceState *dev);
 int qdev_unplug(DeviceState *dev);
 void qdev_free(DeviceState *dev);
-- 
1.6.2.5

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [Qemu-devel] [PATCH 7/7] Clean up test for qdev_init() failure
  2009-10-06 23:15 [Qemu-devel] [PATCH 0/7] Clean up use of qdev_init() Markus Armbruster
                   ` (5 preceding siblings ...)
  2009-10-06 23:16 ` [Qemu-devel] [PATCH 6/7] Warn if value of qdev_init() isn't checked Markus Armbruster
@ 2009-10-06 23:16 ` Markus Armbruster
  6 siblings, 0 replies; 8+ messages in thread
From: Markus Armbruster @ 2009-10-06 23:16 UTC (permalink / raw)
  To: qemu-devel

Some callers test for != 0, some for < 0.  Normalize to < 0.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 hw/fdc.c      |    2 +-
 hw/ide/isa.c  |    2 +-
 hw/ide/qdev.c |    2 +-
 hw/parallel.c |    2 +-
 hw/qdev.c     |    2 +-
 hw/serial.c   |    2 +-
 6 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/hw/fdc.c b/hw/fdc.c
index f1b2f9a..a21e05f 100644
--- a/hw/fdc.c
+++ b/hw/fdc.c
@@ -1847,7 +1847,7 @@ fdctrl_t *fdctrl_init_isa(DriveInfo **fds)
     dev = isa_create("isa-fdc");
     qdev_prop_set_drive(&dev->qdev, "driveA", fds[0]);
     qdev_prop_set_drive(&dev->qdev, "driveB", fds[1]);
-    if (qdev_init(&dev->qdev) != 0)
+    if (qdev_init(&dev->qdev) < 0)
         return NULL;
     return &(DO_UPCAST(fdctrl_isabus_t, busdev, dev)->state);
 }
diff --git a/hw/ide/isa.c b/hw/ide/isa.c
index 3205f40..9f0fdd6 100644
--- a/hw/ide/isa.c
+++ b/hw/ide/isa.c
@@ -85,7 +85,7 @@ int isa_ide_init(int iobase, int iobase2, int isairq,
     qdev_prop_set_uint32(&dev->qdev, "iobase",  iobase);
     qdev_prop_set_uint32(&dev->qdev, "iobase2", iobase2);
     qdev_prop_set_uint32(&dev->qdev, "irq",     isairq);
-    if (qdev_init(&dev->qdev) != 0)
+    if (qdev_init(&dev->qdev) < 0)
         return -1;
 
     s = DO_UPCAST(ISAIDEState, dev, dev);
diff --git a/hw/ide/qdev.c b/hw/ide/qdev.c
index c562bc6..81e7995 100644
--- a/hw/ide/qdev.c
+++ b/hw/ide/qdev.c
@@ -85,7 +85,7 @@ IDEDevice *ide_create_drive(IDEBus *bus, int unit, DriveInfo *drive)
     dev = qdev_create(&bus->qbus, "ide-drive");
     qdev_prop_set_uint32(dev, "unit", unit);
     qdev_prop_set_drive(dev, "drive", drive);
-    if (qdev_init(dev) != 0)
+    if (qdev_init(dev) < 0)
         return NULL;
     return DO_UPCAST(IDEDevice, qdev, dev);
 }
diff --git a/hw/parallel.c b/hw/parallel.c
index 2635edc..92eecb1 100644
--- a/hw/parallel.c
+++ b/hw/parallel.c
@@ -493,7 +493,7 @@ ParallelState *parallel_init(int index, CharDriverState *chr)
     qdev_prop_set_uint32(&dev->qdev, "iobase", isa_parallel_io[index]);
     qdev_prop_set_uint32(&dev->qdev, "irq", 7);
     qdev_prop_set_chr(&dev->qdev, "chardev", chr);
-    if (qdev_init(&dev->qdev) != 0)
+    if (qdev_init(&dev->qdev) < 0)
         return NULL;
     return &DO_UPCAST(ISAParallelState, dev, dev)->state;
 }
diff --git a/hw/qdev.c b/hw/qdev.c
index ca60923..906e897 100644
--- a/hw/qdev.c
+++ b/hw/qdev.c
@@ -214,7 +214,7 @@ DeviceState *qdev_device_add(QemuOpts *opts)
         qdev_free(qdev);
         return NULL;
     }
-    if (qdev_init(qdev) != 0) {
+    if (qdev_init(qdev) < 0) {
         qemu_error("Error initializing device %s\n", driver);
         return NULL;
     }
diff --git a/hw/serial.c b/hw/serial.c
index e044923..eb14f11 100644
--- a/hw/serial.c
+++ b/hw/serial.c
@@ -759,7 +759,7 @@ SerialState *serial_isa_init(int index, CharDriverState *chr)
     qdev_prop_set_uint32(&dev->qdev, "iobase", isa_serial_io[index]);
     qdev_prop_set_uint32(&dev->qdev, "irq", isa_serial_irq[index]);
     qdev_prop_set_chr(&dev->qdev, "chardev", chr);
-    if (qdev_init(&dev->qdev) != 0)
+    if (qdev_init(&dev->qdev) < 0)
         return NULL;
     return &DO_UPCAST(ISASerialState, dev, dev)->state;
 }
-- 
1.6.2.5

^ permalink raw reply related	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2009-10-06 23:16 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2009-10-06 23:15 [Qemu-devel] [PATCH 0/7] Clean up use of qdev_init() Markus Armbruster
2009-10-06 23:15 ` [Qemu-devel] [PATCH 1/7] Unbreak USB autoconnect filters Markus Armbruster
2009-10-06 23:15 ` [Qemu-devel] [PATCH 2/7] Make qdev_init() destroy the device on failure Markus Armbruster
2009-10-06 23:15 ` [Qemu-devel] [PATCH 3/7] Check return value of qdev_init() Markus Armbruster
2009-10-06 23:15 ` [Qemu-devel] [PATCH 4/7] New qdev_init_nofail() Markus Armbruster
2009-10-06 23:15 ` [Qemu-devel] [PATCH 5/7] Make isa_create() terminate program on failure Markus Armbruster
2009-10-06 23:16 ` [Qemu-devel] [PATCH 6/7] Warn if value of qdev_init() isn't checked Markus Armbruster
2009-10-06 23:16 ` [Qemu-devel] [PATCH 7/7] Clean up test for qdev_init() failure Markus Armbruster

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).