Re: [Qemu-devel] [PATCH 09/10] sysbus: add creation function that may fail

[Blue Swirl]

On Sat, Feb 12, 2011 at 7:15 PM, Markus Armbruster <address@hidden> wrote:
> Blue Swirl <address@hidden> writes:
>
>> Signed-off-by: Blue Swirl <address@hidden>
>> ---
>>  hw/sysbus.c |   31 +++++++++++++++++++++++++++++++
>>  hw/sysbus.h |    9 +++++++++
>>  2 files changed, 40 insertions(+), 0 deletions(-)
>>
>> diff --git a/hw/sysbus.c b/hw/sysbus.c
>> index 1583bd8..8980f34 100644
>> --- a/hw/sysbus.c
>> +++ b/hw/sysbus.c
>> @@ -173,6 +173,37 @@ DeviceState *sysbus_create_varargs(const char *name,
>>      return dev;
>>  }
>>
>> +DeviceState *sysbus_try_create_varargs(const char *name,
>> +                                       target_phys_addr_t addr, ...)
>> +{
>> +    DeviceState *dev;
>> +    SysBusDevice *s;
>> +    va_list va;
>> +    qemu_irq irq;
>> +    int n;
>> +
>> +    dev = qdev_try_create(NULL, name);
>> +    if (!dev) {
>> +        return NULL;
>> +    }
>> +    s = sysbus_from_qdev(dev);
>> +    qdev_init_nofail(dev);
>> +    if (addr != (target_phys_addr_t)-1) {
>> +        sysbus_mmio_map(s, 0, addr);
>> +    }
>> +    va_start(va, addr);
>> +    n = 0;
>> +    while (1) {
>> +        irq = va_arg(va, qemu_irq);
>> +        if (!irq) {
>> +            break;
>> +        }
>> +        sysbus_connect_irq(s, n, irq);
>> +        n++;
>> +    }
>> +    return dev;
>> +}
>> +
>
> I really don't like duplicating so much of sysbus_create_varargs()
> here.  Could it be implemented on top of your
> sysbus_try_create_varargs()?

I didn't like this much either, but then there would need to be
another function which takes va_list as a parameter and also error
handling is not so nice.

> Aside: I also don't like the use of variadic arguments for passing IRQ
> numbers.  It's not type-safe.  Why not pass an array and be done with
> it?

Or why not just use the separate property handling functions like the rest?