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

[Markus Armbruster]

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()?

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?

>  static void sysbus_dev_print(Monitor *mon, DeviceState *dev, int indent)
>  {
>      SysBusDevice *s = sysbus_from_qdev(dev);
> diff --git a/hw/sysbus.h b/hw/sysbus.h
> index e9eb618..4e8cb16 100644
> --- a/hw/sysbus.h
> +++ b/hw/sysbus.h
> @@ -57,6 +57,8 @@ void sysbus_mmio_map(SysBusDevice *dev, int n,
> target_phys_addr_t addr);
>  /* Legacy helper function for creating devices.  */
>  DeviceState *sysbus_create_varargs(const char *name,
>                                   target_phys_addr_t addr, ...);
> +DeviceState *sysbus_try_create_varargs(const char *name,
> +                                       target_phys_addr_t addr, ...);
>  static inline DeviceState *sysbus_create_simple(const char *name,
>                                                target_phys_addr_t addr,
>                                                qemu_irq irq)
> @@ -64,4 +66,11 @@ static inline DeviceState
> *sysbus_create_simple(const char *name,
>      return sysbus_create_varargs(name, addr, irq, NULL);
>  }
>
> +static inline DeviceState *sysbus_try_create_simple(const char *name,
> +                                                    target_phys_addr_t addr,
> +                                                    qemu_irq irq)
> +{
> +    return sysbus_try_create_varargs(name, addr, irq, NULL);
> +}
> +
>  #endif /* !HW_SYSBUS_H */