Re: [Qemu-devel] [PATCH 12/16] qdev: Pass size to qbus_create_inplace()

[Andreas Färber]

Am 26.08.2013 09:44, schrieb Wenchao Xia:
> 于 2013-8-24 8:00, Andreas Färber 写道:
>> To be passed to object_initialize().
>>
>> Since commit 39355c3826f5d9a2eb1ce3dc9b4cdd68893769d6 the argument is
>> void*, so drop some superfluous (BusState *) casts or direct parent
>> field usages.
>>
>> Signed-off-by: Andreas Färber <address@hidden>
>> ---
>>   hw/audio/intel-hda.c          | 2 +-
>>   hw/char/ipack.c               | 2 +-
>>   hw/char/virtio-serial-bus.c   | 4 ++--
>>   hw/core/qdev.c                | 2 +-
>>   hw/core/sysbus.c              | 4 ++--
>>   hw/cpu/icc_bus.c              | 3 ++-
>>   hw/ide/qdev.c                 | 2 +-
>>   hw/misc/macio/cuda.c          | 4 ++--
>>   hw/pci/pci.c                  | 2 +-
>>   hw/pci/pci_bridge.c           | 3 ++-
>>   hw/s390x/event-facility.c     | 4 ++--
>>   hw/s390x/s390-virtio-bus.c    | 4 ++--
>>   hw/s390x/virtio-ccw.c         | 4 ++--
>>   hw/scsi/scsi-bus.c            | 2 +-
>>   hw/usb/bus.c                  | 2 +-
>>   hw/usb/dev-smartcard-reader.c | 3 ++-
>>   hw/virtio/virtio-mmio.c       | 2 +-
>>   hw/virtio/virtio-pci.c        | 2 +-
>>   include/hw/qdev-core.h        | 2 +-
>>   19 files changed, 28 insertions(+), 25 deletions(-)
>>
>> diff --git a/hw/audio/intel-hda.c b/hw/audio/intel-hda.c
>> index 9c2fa88..8800dfe 100644
>> --- a/hw/audio/intel-hda.c
>> +++ b/hw/audio/intel-hda.c
>> @@ -44,7 +44,7 @@ void hda_codec_bus_init(DeviceState *dev,
>> HDACodecBus *bus, size_t bus_size,
>>                           hda_codec_response_func response,
>>                           hda_codec_xfer_func xfer)
>>   {
>> -    qbus_create_inplace(&bus->qbus, TYPE_HDA_BUS, dev, NULL);
>> +    qbus_create_inplace(bus, bus_size, TYPE_HDA_BUS, dev, NULL);
>>       bus->response = response;
>>       bus->xfer = xfer;
>>   }
>> diff --git a/hw/char/ipack.c b/hw/char/ipack.c
>> index 5fb7073..b7e45be 100644
>> --- a/hw/char/ipack.c
>> +++ b/hw/char/ipack.c
>> @@ -29,7 +29,7 @@ void ipack_bus_new_inplace(IPackBus *bus, size_t
>> bus_size,
>>                              const char *name, uint8_t n_slots,
>>                              qemu_irq_handler handler)
>>   {
>> -    qbus_create_inplace(&bus->qbus, TYPE_IPACK_BUS, parent, name);
>> +    qbus_create_inplace(bus, bus_size, TYPE_IPACK_BUS, parent, name);
>>       bus->n_slots = n_slots;
>>       bus->set_irq = handler;
>>   }
>> diff --git a/hw/char/virtio-serial-bus.c b/hw/char/virtio-serial-bus.c
>> index da417c7..d90fc2a 100644
>> --- a/hw/char/virtio-serial-bus.c
>> +++ b/hw/char/virtio-serial-bus.c
>> @@ -911,8 +911,8 @@ static int virtio_serial_device_init(VirtIODevice
>> *vdev)
>>                   sizeof(struct virtio_console_config));
>>
>>       /* Spawn a new virtio-serial bus on which the ports will ride as
>> devices */
>> -    qbus_create_inplace(&vser->bus.qbus, TYPE_VIRTIO_SERIAL_BUS, qdev,
>> -                        vdev->bus_name);
>> +    qbus_create_inplace(&vser->bus, sizeof(vser->bus),
>> TYPE_VIRTIO_SERIAL_BUS,
>> +                        qdev, vdev->bus_name);
>>       vser->bus.qbus.allow_hotplug = 1;
>>       vser->bus.vser = vser;
>>       QTAILQ_INIT(&vser->ports);
>> diff --git a/hw/core/qdev.c b/hw/core/qdev.c
>> index 758de9f..81874da 100644
>> --- a/hw/core/qdev.c
>> +++ b/hw/core/qdev.c
>> @@ -470,7 +470,7 @@ static void bus_unparent(Object *obj)
>>       }
>>   }
>>
>> -void qbus_create_inplace(void *bus, const char *typename,
>> +void qbus_create_inplace(void *bus, size_t size, const char *typename,
>>                            DeviceState *parent, const char *name)
>>   {
>>       object_initialize(bus, typename);
>> diff --git a/hw/core/sysbus.c b/hw/core/sysbus.c
>> index 9004d8c..b84cd4a 100644
>> --- a/hw/core/sysbus.c
>> +++ b/hw/core/sysbus.c
>> @@ -276,8 +276,8 @@ static void main_system_bus_create(void)
>>       /* assign main_system_bus before qbus_create_inplace()
>>        * in order to make "if (bus != sysbus_get_default())" work */
>>       main_system_bus = g_malloc0(system_bus_info.instance_size);
>> -    qbus_create_inplace(main_system_bus, TYPE_SYSTEM_BUS, NULL,
>> -                        "main-system-bus");
>> +    qbus_create_inplace(main_system_bus, system_bus_info.instance_size,
>> +                        TYPE_SYSTEM_BUS, NULL, "main-system-bus");
>>       OBJECT(main_system_bus)->free = g_free;
>>       object_property_add_child(container_get(qdev_get_machine(),
>>                                               "/unattached"),
>> diff --git a/hw/cpu/icc_bus.c b/hw/cpu/icc_bus.c
>> index 8748cc5..9a4ea7e 100644
>> --- a/hw/cpu/icc_bus.c
>> +++ b/hw/cpu/icc_bus.c
>> @@ -90,7 +90,8 @@ static void icc_bridge_init(Object *obj)
>>       ICCBridgeState *s = ICC_BRIGDE(obj);
>>       SysBusDevice *sb = SYS_BUS_DEVICE(obj);
>>
>> -    qbus_create_inplace(&s->icc_bus, TYPE_ICC_BUS, DEVICE(s), "icc");
>> +    qbus_create_inplace(&s->icc_bus, sizeof(s->icc_bus), TYPE_ICC_BUS,
>> +                        DEVICE(s), "icc");
>>
>>       /* Do not change order of registering regions,
>>        * APIC must be first registered region, board maps it by 0 index
>> diff --git a/hw/ide/qdev.c b/hw/ide/qdev.c
>> index 8be76ab..18c4b7e 100644
>> --- a/hw/ide/qdev.c
>> +++ b/hw/ide/qdev.c
>> @@ -50,7 +50,7 @@ static const TypeInfo ide_bus_info = {
>>   void ide_bus_new(IDEBus *idebus, size_t idebus_size, DeviceState *dev,
>>                    int bus_id, int max_units)
>>   {
>> -    qbus_create_inplace(&idebus->qbus, TYPE_IDE_BUS, dev, NULL);
>> +    qbus_create_inplace(idebus, idebus_size, TYPE_IDE_BUS, dev, NULL);
>>       idebus->bus_id = bus_id;
>>       idebus->max_units = max_units;
>>   }
>> diff --git a/hw/misc/macio/cuda.c b/hw/misc/macio/cuda.c
>> index c0fd7da..8e41459 100644
>> --- a/hw/misc/macio/cuda.c
>> +++ b/hw/misc/macio/cuda.c
>> @@ -711,8 +711,8 @@ static void cuda_initfn(Object *obj)
>>           s->timers[i].index = i;
>>       }
>>
>> -    qbus_create_inplace((BusState *)&s->adb_bus, TYPE_ADB_BUS,
>> DEVICE(obj),
>> -                        "adb.0");
>> +    qbus_create_inplace(&s->adb_bus, sizeof(s->adb_bus), TYPE_ADB_BUS,
>> +                        DEVICE(obj), "adb.0");
>>   }
>>
>>   static void cuda_class_init(ObjectClass *oc, void *data)
>> diff --git a/hw/pci/pci.c b/hw/pci/pci.c
>> index 397555c..e688f4a 100644
>> --- a/hw/pci/pci.c
>> +++ b/hw/pci/pci.c
>> @@ -318,7 +318,7 @@ void pci_bus_new_inplace(PCIBus *bus, size_t
>> bus_size, DeviceState *parent,
>>                            MemoryRegion *address_space_io,
>>                            uint8_t devfn_min, const char *typename)
>>   {
>> -    qbus_create_inplace(bus, typename, parent, name);
>> +    qbus_create_inplace(bus, bus_size, typename, parent, name);
>>       pci_bus_init(bus, parent, name, address_space_mem,
>>                    address_space_io, devfn_min);
>>   }
>> diff --git a/hw/pci/pci_bridge.c b/hw/pci/pci_bridge.c
>> index a90671d..e6b22b8 100644
>> --- a/hw/pci/pci_bridge.c
>> +++ b/hw/pci/pci_bridge.c
>> @@ -367,7 +367,8 @@ int pci_bridge_initfn(PCIDevice *dev, const char
>> *typename)
>>           br->bus_name = dev->qdev.id;
>>       }
>>
>> -    qbus_create_inplace(&sec_bus->qbus, typename, &dev->qdev,
>> br->bus_name);
>> +    qbus_create_inplace(sec_bus, sizeof(br->sec_bus), typename,
>> DEVICE(dev),
> 
> wouldn't
> qbus_create_inplace(sec_bus, sizeof(*sec_bus), typename, DEVICE(dev),
> looks more straight?

That would defeat part of the purpose. sec_bus is a local variable of
type PCIBus*, to which the actual pointer &br->sec_bus is assigned.
Should that one grow larger, sec_bus can still be used but we don't want
it to assert.

Regards,
Andreas

> 
>> +                        br->bus_name);
>>       sec_bus->parent_dev = dev;
>>       sec_bus->map_irq = br->map_irq ? br->map_irq :
>> pci_swizzle_map_irq_fn;
>>       sec_bus->address_space_mem = &br->address_space_mem;
[snip]

-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg