Convert the hw/input/stellaris_input device to qdev.

The interface uses an array property for the board to specify the
keycodes to use, so the s->keycodes memory is now allocated by the
array-property machinery.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Tested-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
v1->v2: drop private/public comment lines
---
 include/hw/input/stellaris_gamepad.h | 22 ++++++++-
 hw/arm/stellaris.c                   | 26 +++++++---
 hw/input/stellaris_gamepad.c         | 73 +++++++++++++++++++---------
 3 files changed, 89 insertions(+), 32 deletions(-)

diff --git a/include/hw/input/stellaris_gamepad.h b/include/hw/input/stellaris_gamepad.h
index 23cfd3c95f3..6140b889a28 100644
--- a/include/hw/input/stellaris_gamepad.h
+++ b/include/hw/input/stellaris_gamepad.h
@@ -11,8 +11,26 @@
 #ifndef HW_INPUT_STELLARIS_GAMEPAD_H
 #define HW_INPUT_STELLARIS_GAMEPAD_H
 
+#include "hw/sysbus.h"
+#include "qom/object.h"
 
-/* stellaris_gamepad.c */
-void stellaris_gamepad_init(int n, qemu_irq *irq, const int *keycode);
+/*
+ * QEMU interface:
+ *  + QOM array property "keycodes": uint32_t QEMU keycodes to handle
+ *  + unnamed GPIO outputs: one per keycode, in the same order as the
+ *    "keycodes" array property entries; asserted when key is down
+ */
+
+#define TYPE_STELLARIS_GAMEPAD "stellaris-gamepad"
+OBJECT_DECLARE_SIMPLE_TYPE(StellarisGamepad, STELLARIS_GAMEPAD)
+
+struct StellarisGamepad {
+    SysBusDevice parent_obj;
+    uint32_t num_buttons;
+    qemu_irq *irqs;
+    uint32_t *keycodes;
+    uint8_t *pressed;
+    int extension;
+};
 
 #endif
diff --git a/hw/arm/stellaris.c b/hw/arm/stellaris.c
index 96585dd7106..707b0dae375 100644
--- a/hw/arm/stellaris.c
+++ b/hw/arm/stellaris.c
@@ -31,6 +31,7 @@
 #include "hw/timer/stellaris-gptm.h"
 #include "hw/qdev-clock.h"
 #include "qom/object.h"
+#include "qapi/qmp/qlist.h"
 
 #define GPIO_A 0
 #define GPIO_B 1
@@ -1274,16 +1275,27 @@ static void stellaris_init(MachineState *ms, stellaris_board_info *board)
         sysbus_connect_irq(SYS_BUS_DEVICE(enet), 0, qdev_get_gpio_in(nvic, 42));
     }
     if (board->peripherals & BP_GAMEPAD) {
-        qemu_irq gpad_irq[5];
+        QList *gpad_keycode_list = qlist_new();
         static const int gpad_keycode[5] = { 0xc8, 0xd0, 0xcb, 0xcd, 0x1d };
+        DeviceState *gpad;
 
-        gpad_irq[0] = qemu_irq_invert(gpio_in[GPIO_E][0]); /* up */
-        gpad_irq[1] = qemu_irq_invert(gpio_in[GPIO_E][1]); /* down */
-        gpad_irq[2] = qemu_irq_invert(gpio_in[GPIO_E][2]); /* left */
-        gpad_irq[3] = qemu_irq_invert(gpio_in[GPIO_E][3]); /* right */
-        gpad_irq[4] = qemu_irq_invert(gpio_in[GPIO_F][1]); /* select */
+        gpad = qdev_new(TYPE_STELLARIS_GAMEPAD);
+        for (i = 0; i < ARRAY_SIZE(gpad_keycode); i++) {
+            qlist_append_int(gpad_keycode_list, gpad_keycode[i]);
+        }
+        qdev_prop_set_array(gpad, "keycodes", gpad_keycode_list);
+        sysbus_realize_and_unref(SYS_BUS_DEVICE(gpad), &error_fatal);
 
-        stellaris_gamepad_init(5, gpad_irq, gpad_keycode);
+        qdev_connect_gpio_out(gpad, 0,
+                              qemu_irq_invert(gpio_in[GPIO_E][0])); /* up */
+        qdev_connect_gpio_out(gpad, 1,
+                              qemu_irq_invert(gpio_in[GPIO_E][1])); /* down */
+        qdev_connect_gpio_out(gpad, 2,
+                              qemu_irq_invert(gpio_in[GPIO_E][2])); /* left */
+        qdev_connect_gpio_out(gpad, 3,
+                              qemu_irq_invert(gpio_in[GPIO_E][3])); /* right */
+        qdev_connect_gpio_out(gpad, 4,
+                              qemu_irq_invert(gpio_in[GPIO_F][1])); /* select */
     }
     for (i = 0; i < 7; i++) {
         if (board->dc4 & (1 << i)) {
diff --git a/hw/input/stellaris_gamepad.c b/hw/input/stellaris_gamepad.c
index 82ddc47a26d..6ccf0e80adc 100644
--- a/hw/input/stellaris_gamepad.c
+++ b/hw/input/stellaris_gamepad.c
@@ -8,19 +8,13 @@
  */
 
 #include "qemu/osdep.h"
+#include "qapi/error.h"
 #include "hw/input/stellaris_gamepad.h"
 #include "hw/irq.h"
+#include "hw/qdev-properties.h"
 #include "migration/vmstate.h"
 #include "ui/console.h"
 
-typedef struct {
-    uint32_t num_buttons;
-    int extension;
-    qemu_irq *irqs;
-    uint32_t *keycodes;
-    uint8_t *pressed;
-} StellarisGamepad;
-
 static void stellaris_gamepad_put_key(void * opaque, int keycode)
 {
     StellarisGamepad *s = (StellarisGamepad *)opaque;
@@ -57,22 +51,55 @@ static const VMStateDescription vmstate_stellaris_gamepad = {
     }
 };
 
-/* Returns an array of 5 output slots.  */
-void stellaris_gamepad_init(int n, qemu_irq *irq, const int *keycode)
+static void stellaris_gamepad_realize(DeviceState *dev, Error **errp)
 {
-    StellarisGamepad *s;
-    int i;
+    StellarisGamepad *s = STELLARIS_GAMEPAD(dev);
 
-    s = g_new0(StellarisGamepad, 1);
-    s->irqs = g_new0(qemu_irq, n);
-    s->keycodes = g_new0(uint32_t, n);
-    s->pressed = g_new0(uint8_t, n);
-    for (i = 0; i < n; i++) {
-        s->irqs[i] = irq[i];
-        s->keycodes[i] = keycode[i];
+    if (s->num_buttons == 0) {
+        error_setg(errp, "keycodes property array must be set");
+        return;
     }
-    s->num_buttons = n;
-    qemu_add_kbd_event_handler(stellaris_gamepad_put_key, s);
-    vmstate_register(NULL, VMSTATE_INSTANCE_ID_ANY,
-                     &vmstate_stellaris_gamepad, s);
+
+    s->irqs = g_new0(qemu_irq, s->num_buttons);
+    s->pressed = g_new0(uint8_t, s->num_buttons);
+    qdev_init_gpio_out(dev, s->irqs, s->num_buttons);
+    qemu_add_kbd_event_handler(stellaris_gamepad_put_key, dev);
 }
+
+static void stellaris_gamepad_reset_enter(Object *obj, ResetType type)
+{
+    StellarisGamepad *s = STELLARIS_GAMEPAD(obj);
+
+    memset(s->pressed, 0, s->num_buttons * sizeof(uint8_t));
+}
+
+static Property stellaris_gamepad_properties[] = {
+    DEFINE_PROP_ARRAY("keycodes", StellarisGamepad, num_buttons,
+                      keycodes, qdev_prop_uint32, uint32_t),
+    DEFINE_PROP_END_OF_LIST(),
+};
+
+static void stellaris_gamepad_class_init(ObjectClass *klass, void *data)
+{
+    DeviceClass *dc = DEVICE_CLASS(klass);
+    ResettableClass *rc = RESETTABLE_CLASS(klass);
+
+    rc->phases.enter = stellaris_gamepad_reset_enter;
+    dc->realize = stellaris_gamepad_realize;
+    dc->vmsd = &vmstate_stellaris_gamepad;
+    device_class_set_props(dc, stellaris_gamepad_properties);
+}
+
+static const TypeInfo stellaris_gamepad_info = {
+    .name = TYPE_STELLARIS_GAMEPAD,
+    .parent = TYPE_SYS_BUS_DEVICE,
+    .instance_size = sizeof(StellarisGamepad),
+    .class_init = stellaris_gamepad_class_init,
+};
+
+static void stellaris_gamepad_register_types(void)
+{
+    type_register_static(&stellaris_gamepad_info);
+}
+
+type_init(stellaris_gamepad_register_types);
-- 
2.34.1

On 30/10/2023 11:48, Peter Maydell wrote:

> Convert the hw/input/stellaris_input device to qdev.
> 
> The interface uses an array property for the board to specify the
> keycodes to use, so the s->keycodes memory is now allocated by the
> array-property machinery.
> 
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> Tested-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
> v1->v2: drop private/public comment lines
> ---
>   include/hw/input/stellaris_gamepad.h | 22 ++++++++-
>   hw/arm/stellaris.c                   | 26 +++++++---
>   hw/input/stellaris_gamepad.c         | 73 +++++++++++++++++++---------
>   3 files changed, 89 insertions(+), 32 deletions(-)
> 
> diff --git a/include/hw/input/stellaris_gamepad.h b/include/hw/input/stellaris_gamepad.h
> index 23cfd3c95f3..6140b889a28 100644
> --- a/include/hw/input/stellaris_gamepad.h
> +++ b/include/hw/input/stellaris_gamepad.h
> @@ -11,8 +11,26 @@
>   #ifndef HW_INPUT_STELLARIS_GAMEPAD_H
>   #define HW_INPUT_STELLARIS_GAMEPAD_H
>   
> +#include "hw/sysbus.h"
> +#include "qom/object.h"
>   
> -/* stellaris_gamepad.c */
> -void stellaris_gamepad_init(int n, qemu_irq *irq, const int *keycode);
> +/*
> + * QEMU interface:
> + *  + QOM array property "keycodes": uint32_t QEMU keycodes to handle
> + *  + unnamed GPIO outputs: one per keycode, in the same order as the
> + *    "keycodes" array property entries; asserted when key is down
> + */
> +
> +#define TYPE_STELLARIS_GAMEPAD "stellaris-gamepad"
> +OBJECT_DECLARE_SIMPLE_TYPE(StellarisGamepad, STELLARIS_GAMEPAD)
> +
> +struct StellarisGamepad {
> +    SysBusDevice parent_obj;

Minor style comment: for QOM types there should be an empty line after parent_obj to 
aid identification (as per 
https://qemu.readthedocs.io/en/master/devel/style.html#qemu-object-model-declarations).

> +    uint32_t num_buttons;
> +    qemu_irq *irqs;
> +    uint32_t *keycodes;
> +    uint8_t *pressed;
> +    int extension;
> +};
>   
>   #endif
> diff --git a/hw/arm/stellaris.c b/hw/arm/stellaris.c
> index 96585dd7106..707b0dae375 100644
> --- a/hw/arm/stellaris.c
> +++ b/hw/arm/stellaris.c
> @@ -31,6 +31,7 @@
>   #include "hw/timer/stellaris-gptm.h"
>   #include "hw/qdev-clock.h"
>   #include "qom/object.h"
> +#include "qapi/qmp/qlist.h"
>   
>   #define GPIO_A 0
>   #define GPIO_B 1
> @@ -1274,16 +1275,27 @@ static void stellaris_init(MachineState *ms, stellaris_board_info *board)
>           sysbus_connect_irq(SYS_BUS_DEVICE(enet), 0, qdev_get_gpio_in(nvic, 42));
>       }
>       if (board->peripherals & BP_GAMEPAD) {
> -        qemu_irq gpad_irq[5];
> +        QList *gpad_keycode_list = qlist_new();
>           static const int gpad_keycode[5] = { 0xc8, 0xd0, 0xcb, 0xcd, 0x1d };
> +        DeviceState *gpad;
>   
> -        gpad_irq[0] = qemu_irq_invert(gpio_in[GPIO_E][0]); /* up */
> -        gpad_irq[1] = qemu_irq_invert(gpio_in[GPIO_E][1]); /* down */
> -        gpad_irq[2] = qemu_irq_invert(gpio_in[GPIO_E][2]); /* left */
> -        gpad_irq[3] = qemu_irq_invert(gpio_in[GPIO_E][3]); /* right */
> -        gpad_irq[4] = qemu_irq_invert(gpio_in[GPIO_F][1]); /* select */
> +        gpad = qdev_new(TYPE_STELLARIS_GAMEPAD);
> +        for (i = 0; i < ARRAY_SIZE(gpad_keycode); i++) {
> +            qlist_append_int(gpad_keycode_list, gpad_keycode[i]);
> +        }
> +        qdev_prop_set_array(gpad, "keycodes", gpad_keycode_list);
> +        sysbus_realize_and_unref(SYS_BUS_DEVICE(gpad), &error_fatal);
>   
> -        stellaris_gamepad_init(5, gpad_irq, gpad_keycode);
> +        qdev_connect_gpio_out(gpad, 0,
> +                              qemu_irq_invert(gpio_in[GPIO_E][0])); /* up */
> +        qdev_connect_gpio_out(gpad, 1,
> +                              qemu_irq_invert(gpio_in[GPIO_E][1])); /* down */
> +        qdev_connect_gpio_out(gpad, 2,
> +                              qemu_irq_invert(gpio_in[GPIO_E][2])); /* left */
> +        qdev_connect_gpio_out(gpad, 3,
> +                              qemu_irq_invert(gpio_in[GPIO_E][3])); /* right */
> +        qdev_connect_gpio_out(gpad, 4,
> +                              qemu_irq_invert(gpio_in[GPIO_F][1])); /* select */
>       }
>       for (i = 0; i < 7; i++) {
>           if (board->dc4 & (1 << i)) {
> diff --git a/hw/input/stellaris_gamepad.c b/hw/input/stellaris_gamepad.c
> index 82ddc47a26d..6ccf0e80adc 100644
> --- a/hw/input/stellaris_gamepad.c
> +++ b/hw/input/stellaris_gamepad.c
> @@ -8,19 +8,13 @@
>    */
>   
>   #include "qemu/osdep.h"
> +#include "qapi/error.h"
>   #include "hw/input/stellaris_gamepad.h"
>   #include "hw/irq.h"
> +#include "hw/qdev-properties.h"
>   #include "migration/vmstate.h"
>   #include "ui/console.h"
>   
> -typedef struct {
> -    uint32_t num_buttons;
> -    int extension;
> -    qemu_irq *irqs;
> -    uint32_t *keycodes;
> -    uint8_t *pressed;
> -} StellarisGamepad;
> -
>   static void stellaris_gamepad_put_key(void * opaque, int keycode)
>   {
>       StellarisGamepad *s = (StellarisGamepad *)opaque;
> @@ -57,22 +51,55 @@ static const VMStateDescription vmstate_stellaris_gamepad = {
>       }
>   };
>   
> -/* Returns an array of 5 output slots.  */
> -void stellaris_gamepad_init(int n, qemu_irq *irq, const int *keycode)
> +static void stellaris_gamepad_realize(DeviceState *dev, Error **errp)
>   {
> -    StellarisGamepad *s;
> -    int i;
> +    StellarisGamepad *s = STELLARIS_GAMEPAD(dev);
>   
> -    s = g_new0(StellarisGamepad, 1);
> -    s->irqs = g_new0(qemu_irq, n);
> -    s->keycodes = g_new0(uint32_t, n);
> -    s->pressed = g_new0(uint8_t, n);
> -    for (i = 0; i < n; i++) {
> -        s->irqs[i] = irq[i];
> -        s->keycodes[i] = keycode[i];
> +    if (s->num_buttons == 0) {
> +        error_setg(errp, "keycodes property array must be set");
> +        return;
>       }
> -    s->num_buttons = n;
> -    qemu_add_kbd_event_handler(stellaris_gamepad_put_key, s);
> -    vmstate_register(NULL, VMSTATE_INSTANCE_ID_ANY,
> -                     &vmstate_stellaris_gamepad, s);
> +
> +    s->irqs = g_new0(qemu_irq, s->num_buttons);
> +    s->pressed = g_new0(uint8_t, s->num_buttons);
> +    qdev_init_gpio_out(dev, s->irqs, s->num_buttons);
> +    qemu_add_kbd_event_handler(stellaris_gamepad_put_key, dev);
>   }
> +
> +static void stellaris_gamepad_reset_enter(Object *obj, ResetType type)
> +{
> +    StellarisGamepad *s = STELLARIS_GAMEPAD(obj);
> +
> +    memset(s->pressed, 0, s->num_buttons * sizeof(uint8_t));
> +}
> +
> +static Property stellaris_gamepad_properties[] = {
> +    DEFINE_PROP_ARRAY("keycodes", StellarisGamepad, num_buttons,
> +                      keycodes, qdev_prop_uint32, uint32_t),
> +    DEFINE_PROP_END_OF_LIST(),
> +};
> +
> +static void stellaris_gamepad_class_init(ObjectClass *klass, void *data)
> +{
> +    DeviceClass *dc = DEVICE_CLASS(klass);
> +    ResettableClass *rc = RESETTABLE_CLASS(klass);
> +
> +    rc->phases.enter = stellaris_gamepad_reset_enter;
> +    dc->realize = stellaris_gamepad_realize;
> +    dc->vmsd = &vmstate_stellaris_gamepad;
> +    device_class_set_props(dc, stellaris_gamepad_properties);
> +}
> +
> +static const TypeInfo stellaris_gamepad_info = {
> +    .name = TYPE_STELLARIS_GAMEPAD,
> +    .parent = TYPE_SYS_BUS_DEVICE,
> +    .instance_size = sizeof(StellarisGamepad),
> +    .class_init = stellaris_gamepad_class_init,
> +};
> +
> +static void stellaris_gamepad_register_types(void)
> +{
> +    type_register_static(&stellaris_gamepad_info);
> +}
> +
> +type_init(stellaris_gamepad_register_types);

Is it worth converting this to use DEFINE_TYPES() during the conversion? I know Phil 
has considered some automation to remove the type_init() boilerplate for the majority 
of cases.


ATB,

Mark.

On Mon, 30 Oct 2023 at 20:38, Mark Cave-Ayland
<mark.cave-ayland@ilande.co.uk> wrote:
>
> On 30/10/2023 11:48, Peter Maydell wrote:
>
> > Convert the hw/input/stellaris_input device to qdev.
> >
> > The interface uses an array property for the board to specify the
> > keycodes to use, so the s->keycodes memory is now allocated by the
> > array-property machinery.
> >
> > Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> > Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> > Tested-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> > ---
> > v1->v2: drop private/public comment lines
> > ---
> >   include/hw/input/stellaris_gamepad.h | 22 ++++++++-
> >   hw/arm/stellaris.c                   | 26 +++++++---
> >   hw/input/stellaris_gamepad.c         | 73 +++++++++++++++++++---------
> >   3 files changed, 89 insertions(+), 32 deletions(-)
> >
> > diff --git a/include/hw/input/stellaris_gamepad.h b/include/hw/input/stellaris_gamepad.h
> > index 23cfd3c95f3..6140b889a28 100644
> > --- a/include/hw/input/stellaris_gamepad.h
> > +++ b/include/hw/input/stellaris_gamepad.h
> > @@ -11,8 +11,26 @@
> >   #ifndef HW_INPUT_STELLARIS_GAMEPAD_H
> >   #define HW_INPUT_STELLARIS_GAMEPAD_H
> >
> > +#include "hw/sysbus.h"
> > +#include "qom/object.h"
> >
> > -/* stellaris_gamepad.c */
> > -void stellaris_gamepad_init(int n, qemu_irq *irq, const int *keycode);
> > +/*
> > + * QEMU interface:
> > + *  + QOM array property "keycodes": uint32_t QEMU keycodes to handle
> > + *  + unnamed GPIO outputs: one per keycode, in the same order as the
> > + *    "keycodes" array property entries; asserted when key is down
> > + */
> > +
> > +#define TYPE_STELLARIS_GAMEPAD "stellaris-gamepad"
> > +OBJECT_DECLARE_SIMPLE_TYPE(StellarisGamepad, STELLARIS_GAMEPAD)
> > +
> > +struct StellarisGamepad {
> > +    SysBusDevice parent_obj;
>
> Minor style comment: for QOM types there should be an empty line after parent_obj to
> aid identification (as per
> https://qemu.readthedocs.io/en/master/devel/style.html#qemu-object-model-declarations).

Fixed.

> > +static const TypeInfo stellaris_gamepad_info = {
> > +    .name = TYPE_STELLARIS_GAMEPAD,
> > +    .parent = TYPE_SYS_BUS_DEVICE,
> > +    .instance_size = sizeof(StellarisGamepad),
> > +    .class_init = stellaris_gamepad_class_init,
> > +};
> > +
> > +static void stellaris_gamepad_register_types(void)
> > +{
> > +    type_register_static(&stellaris_gamepad_info);
> > +}
> > +
> > +type_init(stellaris_gamepad_register_types);
>
> Is it worth converting this to use DEFINE_TYPES() during the conversion? I know Phil
> has considered some automation to remove the type_init() boilerplate for the majority
> of cases.

I could, I guess. It seems a bit awkward that DEFINE_TYPES()
wants you to pass it an array even when you only have one type,
though, which is going to be a very common use case.

-- PMM

On Tue, 31 Oct 2023 at 13:55, Peter Maydell <peter.maydell@linaro.org> wrote:
>
> On Mon, 30 Oct 2023 at 20:38, Mark Cave-Ayland
> <mark.cave-ayland@ilande.co.uk> wrote:
> >
> > On 30/10/2023 11:48, Peter Maydell wrote:
> > Is it worth converting this to use DEFINE_TYPES() during the conversion? I know Phil
> > has considered some automation to remove the type_init() boilerplate for the majority
> > of cases.
>
> I could, I guess. It seems a bit awkward that DEFINE_TYPES()
> wants you to pass it an array even when you only have one type,
> though, which is going to be a very common use case.

I'm going to squash this into this patch:
diff --git a/hw/input/stellaris_gamepad.c b/hw/input/stellaris_gamepad.c
index 6ccf0e80adc..d42ba4f0582 100644
--- a/hw/input/stellaris_gamepad.c
+++ b/hw/input/stellaris_gamepad.c
@@ -90,16 +90,13 @@ static void
stellaris_gamepad_class_init(ObjectClass *klass, void *data)
     device_class_set_props(dc, stellaris_gamepad_properties);
 }

-static const TypeInfo stellaris_gamepad_info = {
-    .name = TYPE_STELLARIS_GAMEPAD,
-    .parent = TYPE_SYS_BUS_DEVICE,
-    .instance_size = sizeof(StellarisGamepad),
-    .class_init = stellaris_gamepad_class_init,
+static const TypeInfo stellaris_gamepad_info[] = {
+    {
+        .name = TYPE_STELLARIS_GAMEPAD,
+        .parent = TYPE_SYS_BUS_DEVICE,
+        .instance_size = sizeof(StellarisGamepad),
+        .class_init = stellaris_gamepad_class_init,
+    },
 };

-static void stellaris_gamepad_register_types(void)
-{
-    type_register_static(&stellaris_gamepad_info);
-}
-
-type_init(stellaris_gamepad_register_types);
+DEFINE_TYPES(stellaris_gamepad_info);


The array is a bit awkward, but it's overall better than having
to define the register-types function.

thanks
-- PMM

On 31/10/23 15:05, Peter Maydell wrote:
> On Tue, 31 Oct 2023 at 13:55, Peter Maydell <peter.maydell@linaro.org> wrote:
>>
>> On Mon, 30 Oct 2023 at 20:38, Mark Cave-Ayland
>> <mark.cave-ayland@ilande.co.uk> wrote:
>>>
>>> On 30/10/2023 11:48, Peter Maydell wrote:
>>> Is it worth converting this to use DEFINE_TYPES() during the conversion? I know Phil
>>> has considered some automation to remove the type_init() boilerplate for the majority
>>> of cases.
>>
>> I could, I guess. It seems a bit awkward that DEFINE_TYPES()
>> wants you to pass it an array even when you only have one type,
>> though, which is going to be a very common use case.

For single type, there is no point beside enforcing a QOM style.

I'll update docs/devel/qom.rst...

> I'm going to squash this into this patch:
> diff --git a/hw/input/stellaris_gamepad.c b/hw/input/stellaris_gamepad.c
> index 6ccf0e80adc..d42ba4f0582 100644
> --- a/hw/input/stellaris_gamepad.c
> +++ b/hw/input/stellaris_gamepad.c
> @@ -90,16 +90,13 @@ static void
> stellaris_gamepad_class_init(ObjectClass *klass, void *data)
>       device_class_set_props(dc, stellaris_gamepad_properties);
>   }
> 
> -static const TypeInfo stellaris_gamepad_info = {
> -    .name = TYPE_STELLARIS_GAMEPAD,
> -    .parent = TYPE_SYS_BUS_DEVICE,
> -    .instance_size = sizeof(StellarisGamepad),
> -    .class_init = stellaris_gamepad_class_init,
> +static const TypeInfo stellaris_gamepad_info[] = {
> +    {
> +        .name = TYPE_STELLARIS_GAMEPAD,
> +        .parent = TYPE_SYS_BUS_DEVICE,
> +        .instance_size = sizeof(StellarisGamepad),
> +        .class_init = stellaris_gamepad_class_init,
> +    },
>   };
> 
> -static void stellaris_gamepad_register_types(void)
> -{
> -    type_register_static(&stellaris_gamepad_info);
> -}
> -
> -type_init(stellaris_gamepad_register_types);
> +DEFINE_TYPES(stellaris_gamepad_info);
> 
> 
> The array is a bit awkward, but it's overall better than having
> to define the register-types function.

Thank you!

On Tue, 31 Oct 2023 at 14:55, Philippe Mathieu-Daudé <philmd@linaro.org> wrote:
>
> On 31/10/23 15:05, Peter Maydell wrote:
> > On Tue, 31 Oct 2023 at 13:55, Peter Maydell <peter.maydell@linaro.org> wrote:
> >>
> >> On Mon, 30 Oct 2023 at 20:38, Mark Cave-Ayland
> >> <mark.cave-ayland@ilande.co.uk> wrote:
> >>>
> >>> On 30/10/2023 11:48, Peter Maydell wrote:
> >>> Is it worth converting this to use DEFINE_TYPES() during the conversion? I know Phil
> >>> has considered some automation to remove the type_init() boilerplate for the majority
> >>> of cases.
> >>
> >> I could, I guess. It seems a bit awkward that DEFINE_TYPES()
> >> wants you to pass it an array even when you only have one type,
> >> though, which is going to be a very common use case.
>
> For single type, there is no point beside enforcing a QOM style.
>
> I'll update docs/devel/qom.rst...

I do like that the macro means you're not writing an actual
function for the registration.

We could I guess have a DEFINE_TYPE() macro that was similar
to DEFINE_TYPES but emitted a function that called
type_register_static() instead of type_register_static_array().
Is that worth having? I'm not sure.

thanks
-- PMM

Hi Peter,

Cc'ing Markus for QObject.

On 30/10/23 12:48, Peter Maydell wrote:
> Convert the hw/input/stellaris_input device to qdev.
> 
> The interface uses an array property for the board to specify the
> keycodes to use, so the s->keycodes memory is now allocated by the
> array-property machinery.
> 
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> Tested-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
> v1->v2: drop private/public comment lines
> ---
>   include/hw/input/stellaris_gamepad.h | 22 ++++++++-
>   hw/arm/stellaris.c                   | 26 +++++++---
>   hw/input/stellaris_gamepad.c         | 73 +++++++++++++++++++---------
>   3 files changed, 89 insertions(+), 32 deletions(-)


> diff --git a/hw/arm/stellaris.c b/hw/arm/stellaris.c
> index 96585dd7106..707b0dae375 100644
> --- a/hw/arm/stellaris.c
> +++ b/hw/arm/stellaris.c
> @@ -31,6 +31,7 @@
>   #include "hw/timer/stellaris-gptm.h"
>   #include "hw/qdev-clock.h"
>   #include "qom/object.h"
> +#include "qapi/qmp/qlist.h"
>   
>   #define GPIO_A 0
>   #define GPIO_B 1
> @@ -1274,16 +1275,27 @@ static void stellaris_init(MachineState *ms, stellaris_board_info *board)
>           sysbus_connect_irq(SYS_BUS_DEVICE(enet), 0, qdev_get_gpio_in(nvic, 42));
>       }
>       if (board->peripherals & BP_GAMEPAD) {
> -        qemu_irq gpad_irq[5];
> +        QList *gpad_keycode_list = qlist_new();

I'm trying to understand better qlist_new(), but unfortunately there
is not much documentation. Looking at how the allocated list was
released, I found use of g_autoptr in tests/unit/check-qobject.c,
so I tried:

            g_autoptr(QList) gpad_keycode_list = qlist_new();

But QEMU crashes elsewhere which seems unrelated:

* thread #2, stop reason = signal SIGABRT
   * frame #0: 0x8b1eb11c libsystem_kernel.dylib`__pthread_kill + 8
     frame #1: 0x8b222cc0 libsystem_pthread.dylib`pthread_kill + 288
     frame #2: 0x8b132a50 libsystem_c.dylib`abort + 180
     frame #3: 0x8b049b08 libsystem_malloc.dylib`malloc_vreport + 908
     frame #4: 0x8b06924c libsystem_malloc.dylib`malloc_zone_error + 104
     frame #5: 0x8b05b094 
libsystem_malloc.dylib`nanov2_guard_corruption_detected + 44
     frame #6: 0x8b05a2a8 
libsystem_malloc.dylib`nanov2_allocate_outlined + 404
     frame #7: 0x0201fdc0 libglib-2.0.0.dylib`g_malloc0 + 36
     frame #8: 0x02007718 libglib-2.0.0.dylib`g_hash_table_setup_storage 
+ 76
     frame #9: 0x020076b0 libglib-2.0.0.dylib`g_hash_table_new_full + 96
     frame #10: 0x003a9920 qemu-system-ppc`object_unref [inlined] 
object_property_del_all(obj=0x42023e00) at object.c:635:34
     frame #11: 0x003a9914 qemu-system-ppc`object_unref [inlined] 
object_finalize(data=0x42023e00) at object.c:707:5
     frame #12: 0x003a990c 
qemu-system-ppc`object_unref(objptr=0x42023e00) at object.c:1216:9
     frame #13: 0x00355114 qemu-system-ppc`address_space_dispatch_free 
at physmem.c:1001:9
     frame #14: 0x003550fc qemu-system-ppc`address_space_dispatch_free 
at physmem.c:1010:9
     frame #15: 0x003550e0 
qemu-system-ppc`address_space_dispatch_free(d=0x000060000385d680) at 
physmem.c:2473:5
     frame #16: 0x00349438 
qemu-system-ppc`flatview_destroy(view=0x000060000385d640) at memory.c:295:9
     frame #17: 0x00524920 
qemu-system-ppc`call_rcu_thread(opaque=<unavailable>) at rcu.c:301:13
     frame #18: 0x0051c1f0 
qemu-system-ppc`qemu_thread_start(args=<unavailable>) at 
qemu-thread-posix.c:541:9
     frame #19: 0x8b223034 libsystem_pthread.dylib`_pthread_start + 136

However when running 'make check-unit', qobject_is_equal_list_test()
is successful, so I'm a bit confused...

>           static const int gpad_keycode[5] = { 0xc8, 0xd0, 0xcb, 0xcd, 0x1d };
> +        DeviceState *gpad;
>   
> -        gpad_irq[0] = qemu_irq_invert(gpio_in[GPIO_E][0]); /* up */
> -        gpad_irq[1] = qemu_irq_invert(gpio_in[GPIO_E][1]); /* down */
> -        gpad_irq[2] = qemu_irq_invert(gpio_in[GPIO_E][2]); /* left */
> -        gpad_irq[3] = qemu_irq_invert(gpio_in[GPIO_E][3]); /* right */
> -        gpad_irq[4] = qemu_irq_invert(gpio_in[GPIO_F][1]); /* select */
> +        gpad = qdev_new(TYPE_STELLARIS_GAMEPAD);
> +        for (i = 0; i < ARRAY_SIZE(gpad_keycode); i++) {
> +            qlist_append_int(gpad_keycode_list, gpad_keycode[i]);
> +        }
> +        qdev_prop_set_array(gpad, "keycodes", gpad_keycode_list);
> +        sysbus_realize_and_unref(SYS_BUS_DEVICE(gpad), &error_fatal);
>   
> -        stellaris_gamepad_init(5, gpad_irq, gpad_keycode);
> +        qdev_connect_gpio_out(gpad, 0,
> +                              qemu_irq_invert(gpio_in[GPIO_E][0])); /* up */
> +        qdev_connect_gpio_out(gpad, 1,
> +                              qemu_irq_invert(gpio_in[GPIO_E][1])); /* down */
> +        qdev_connect_gpio_out(gpad, 2,
> +                              qemu_irq_invert(gpio_in[GPIO_E][2])); /* left */
> +        qdev_connect_gpio_out(gpad, 3,
> +                              qemu_irq_invert(gpio_in[GPIO_E][3])); /* right */
> +        qdev_connect_gpio_out(gpad, 4,
> +                              qemu_irq_invert(gpio_in[GPIO_F][1])); /* select */
>       }

On Mon, 30 Oct 2023 at 13:52, Philippe Mathieu-Daudé <philmd@linaro.org> wrote:
>
> Hi Peter,
>
> Cc'ing Markus for QObject.
>
> On 30/10/23 12:48, Peter Maydell wrote:
> > Convert the hw/input/stellaris_input device to qdev.
> >
> > The interface uses an array property for the board to specify the
> > keycodes to use, so the s->keycodes memory is now allocated by the
> > array-property machinery.
> >
> > Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> > Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> > Tested-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> > ---
> > v1->v2: drop private/public comment lines
> > ---
> >   include/hw/input/stellaris_gamepad.h | 22 ++++++++-
> >   hw/arm/stellaris.c                   | 26 +++++++---
> >   hw/input/stellaris_gamepad.c         | 73 +++++++++++++++++++---------
> >   3 files changed, 89 insertions(+), 32 deletions(-)
>
>
> > diff --git a/hw/arm/stellaris.c b/hw/arm/stellaris.c
> > index 96585dd7106..707b0dae375 100644
> > --- a/hw/arm/stellaris.c
> > +++ b/hw/arm/stellaris.c
> > @@ -31,6 +31,7 @@
> >   #include "hw/timer/stellaris-gptm.h"
> >   #include "hw/qdev-clock.h"
> >   #include "qom/object.h"
> > +#include "qapi/qmp/qlist.h"
> >
> >   #define GPIO_A 0
> >   #define GPIO_B 1
> > @@ -1274,16 +1275,27 @@ static void stellaris_init(MachineState *ms, stellaris_board_info *board)
> >           sysbus_connect_irq(SYS_BUS_DEVICE(enet), 0, qdev_get_gpio_in(nvic, 42));
> >       }
> >       if (board->peripherals & BP_GAMEPAD) {
> > -        qemu_irq gpad_irq[5];
> > +        QList *gpad_keycode_list = qlist_new();
>
> I'm trying to understand better qlist_new(), but unfortunately there
> is not much documentation. Looking at how the allocated list was
> released, I found use of g_autoptr in tests/unit/check-qobject.c,
> so I tried:
>
>             g_autoptr(QList) gpad_keycode_list = qlist_new();

The API for qdev_prop_set_array() documents that it takes ownership
of the list you pass it (and it ends up calling qobject_unref() on it).
So I think adding g_autoptr() here will result in the memory being
double-freed (once inside qobject_unref() when the refcount
goes to 0, and once when g_autoptr frees it as it goes out of scope)...

> * thread #2, stop reason = signal SIGABRT
>    * frame #0: 0x8b1eb11c libsystem_kernel.dylib`__pthread_kill + 8
>      frame #1: 0x8b222cc0 libsystem_pthread.dylib`pthread_kill + 288
>      frame #2: 0x8b132a50 libsystem_c.dylib`abort + 180
>      frame #3: 0x8b049b08 libsystem_malloc.dylib`malloc_vreport + 908
>      frame #4: 0x8b06924c libsystem_malloc.dylib`malloc_zone_error + 104
>      frame #5: 0x8b05b094
> libsystem_malloc.dylib`nanov2_guard_corruption_detected + 44
>      frame #6: 0x8b05a2a8
> libsystem_malloc.dylib`nanov2_allocate_outlined + 404
>      frame #7: 0x0201fdc0 libglib-2.0.0.dylib`g_malloc0 + 36
>      frame #8: 0x02007718 libglib-2.0.0.dylib`g_hash_table_setup_storage
> + 76
>      frame #9: 0x020076b0 libglib-2.0.0.dylib`g_hash_table_new_full + 96

...which is probably why a later memory operation runs into a
malloc data corruption assertion.

thanks
-- PMM

On 30/10/23 15:41, Peter Maydell wrote:
> On Mon, 30 Oct 2023 at 13:52, Philippe Mathieu-Daudé <philmd@linaro.org> wrote:
>>
>> Hi Peter,
>>
>> Cc'ing Markus for QObject.
>>
>> On 30/10/23 12:48, Peter Maydell wrote:
>>> Convert the hw/input/stellaris_input device to qdev.
>>>
>>> The interface uses an array property for the board to specify the
>>> keycodes to use, so the s->keycodes memory is now allocated by the
>>> array-property machinery.
>>>
>>> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
>>> Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>>> Tested-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>>> ---
>>> v1->v2: drop private/public comment lines
>>> ---
>>>    include/hw/input/stellaris_gamepad.h | 22 ++++++++-
>>>    hw/arm/stellaris.c                   | 26 +++++++---
>>>    hw/input/stellaris_gamepad.c         | 73 +++++++++++++++++++---------
>>>    3 files changed, 89 insertions(+), 32 deletions(-)
>>
>>
>>> diff --git a/hw/arm/stellaris.c b/hw/arm/stellaris.c
>>> index 96585dd7106..707b0dae375 100644
>>> --- a/hw/arm/stellaris.c
>>> +++ b/hw/arm/stellaris.c
>>> @@ -31,6 +31,7 @@
>>>    #include "hw/timer/stellaris-gptm.h"
>>>    #include "hw/qdev-clock.h"
>>>    #include "qom/object.h"
>>> +#include "qapi/qmp/qlist.h"
>>>
>>>    #define GPIO_A 0
>>>    #define GPIO_B 1
>>> @@ -1274,16 +1275,27 @@ static void stellaris_init(MachineState *ms, stellaris_board_info *board)
>>>            sysbus_connect_irq(SYS_BUS_DEVICE(enet), 0, qdev_get_gpio_in(nvic, 42));
>>>        }
>>>        if (board->peripherals & BP_GAMEPAD) {
>>> -        qemu_irq gpad_irq[5];
>>> +        QList *gpad_keycode_list = qlist_new();
>>
>> I'm trying to understand better qlist_new(), but unfortunately there
>> is not much documentation. Looking at how the allocated list was
>> released, I found use of g_autoptr in tests/unit/check-qobject.c,
>> so I tried:
>>
>>              g_autoptr(QList) gpad_keycode_list = qlist_new();
> 
> The API for qdev_prop_set_array() documents that it takes ownership
> of the list you pass it (and it ends up calling qobject_unref() on it).
> So I think adding g_autoptr() here will result in the memory being
> double-freed (once inside qobject_unref() when the refcount
> goes to 0, and once when g_autoptr frees it as it goes out of scope)...

Ah, I missed how qdev_prop_set_array() is involved.

>> * thread #2, stop reason = signal SIGABRT
>>     * frame #0: 0x8b1eb11c libsystem_kernel.dylib`__pthread_kill + 8
>>       frame #1: 0x8b222cc0 libsystem_pthread.dylib`pthread_kill + 288
>>       frame #2: 0x8b132a50 libsystem_c.dylib`abort + 180
>>       frame #3: 0x8b049b08 libsystem_malloc.dylib`malloc_vreport + 908
>>       frame #4: 0x8b06924c libsystem_malloc.dylib`malloc_zone_error + 104
>>       frame #5: 0x8b05b094
>> libsystem_malloc.dylib`nanov2_guard_corruption_detected + 44
>>       frame #6: 0x8b05a2a8
>> libsystem_malloc.dylib`nanov2_allocate_outlined + 404
>>       frame #7: 0x0201fdc0 libglib-2.0.0.dylib`g_malloc0 + 36
>>       frame #8: 0x02007718 libglib-2.0.0.dylib`g_hash_table_setup_storage
>> + 76
>>       frame #9: 0x020076b0 libglib-2.0.0.dylib`g_hash_table_new_full + 96
> 
> ...which is probably why a later memory operation runs into a
> malloc data corruption assertion.

Yes, this is certainly the reason. Thanks for the explanation!

Phil.

Philippe Mathieu-Daudé <philmd@linaro.org> writes:

> Hi Peter,
>
> Cc'ing Markus for QObject.
>
> On 30/10/23 12:48, Peter Maydell wrote:
>> Convert the hw/input/stellaris_input device to qdev.
>> The interface uses an array property for the board to specify the
>> keycodes to use, so the s->keycodes memory is now allocated by the
>> array-property machinery.
>> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
>> Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>> Tested-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>> ---
>> v1->v2: drop private/public comment lines
>> ---
>>   include/hw/input/stellaris_gamepad.h | 22 ++++++++-
>>   hw/arm/stellaris.c                   | 26 +++++++---
>>   hw/input/stellaris_gamepad.c         | 73 +++++++++++++++++++---------
>>   3 files changed, 89 insertions(+), 32 deletions(-)
>
>
>> diff --git a/hw/arm/stellaris.c b/hw/arm/stellaris.c
>> index 96585dd7106..707b0dae375 100644
>> --- a/hw/arm/stellaris.c
>> +++ b/hw/arm/stellaris.c
>> @@ -31,6 +31,7 @@
>>   #include "hw/timer/stellaris-gptm.h"
>>   #include "hw/qdev-clock.h"
>>   #include "qom/object.h"
>> +#include "qapi/qmp/qlist.h"
>>     #define GPIO_A 0
>>   #define GPIO_B 1
>> @@ -1274,16 +1275,27 @@ static void stellaris_init(MachineState *ms, stellaris_board_info *board)
>>           sysbus_connect_irq(SYS_BUS_DEVICE(enet), 0, qdev_get_gpio_in(nvic, 42));
>>       }
>>       if (board->peripherals & BP_GAMEPAD) {
>> -        qemu_irq gpad_irq[5];
>> +        QList *gpad_keycode_list = qlist_new();
>
> I'm trying to understand better qlist_new(), but unfortunately there
> is not much documentation. Looking at how the allocated list was
> released, I found use of g_autoptr in tests/unit/check-qobject.c,
> so I tried:
>
>            g_autoptr(QList) gpad_keycode_list = qlist_new();

QObject and its subtypes QDict, QList, QString, ... are reference
counted.  qFOO_new() ist to be paired with qFOO_unref() or
qobject_unref().

Your use of g_autoptr(QList) should work.

> But QEMU crashes elsewhere which seems unrelated:
>
> * thread #2, stop reason = signal SIGABRT
>   * frame #0: 0x8b1eb11c libsystem_kernel.dylib`__pthread_kill + 8
>     frame #1: 0x8b222cc0 libsystem_pthread.dylib`pthread_kill + 288
>     frame #2: 0x8b132a50 libsystem_c.dylib`abort + 180
>     frame #3: 0x8b049b08 libsystem_malloc.dylib`malloc_vreport + 908
>     frame #4: 0x8b06924c libsystem_malloc.dylib`malloc_zone_error + 104
>     frame #5: 0x8b05b094 libsystem_malloc.dylib`nanov2_guard_corruption_detected + 44
>     frame #6: 0x8b05a2a8 libsystem_malloc.dylib`nanov2_allocate_outlined + 404
>     frame #7: 0x0201fdc0 libglib-2.0.0.dylib`g_malloc0 + 36
>     frame #8: 0x02007718 libglib-2.0.0.dylib`g_hash_table_setup_storage + 76
>     frame #9: 0x020076b0 libglib-2.0.0.dylib`g_hash_table_new_full + 96
>     frame #10: 0x003a9920 qemu-system-ppc`object_unref [inlined] object_property_del_all(obj=0x42023e00) at object.c:635:34
>     frame #11: 0x003a9914 qemu-system-ppc`object_unref [inlined] object_finalize(data=0x42023e00) at object.c:707:5
>     frame #12: 0x003a990c qemu-system-ppc`object_unref(objptr=0x42023e00) at object.c:1216:9
>     frame #13: 0x00355114 qemu-system-ppc`address_space_dispatch_free at physmem.c:1001:9
>     frame #14: 0x003550fc qemu-system-ppc`address_space_dispatch_free at physmem.c:1010:9
>     frame #15: 0x003550e0 qemu-system-ppc`address_space_dispatch_free(d=0x000060000385d680) at physmem.c:2473:5
>     frame #16: 0x00349438 qemu-system-ppc`flatview_destroy(view=0x000060000385d640) at memory.c:295:9
>     frame #17: 0x00524920 qemu-system-ppc`call_rcu_thread(opaque=<unavailable>) at rcu.c:301:13
>     frame #18: 0x0051c1f0 qemu-system-ppc`qemu_thread_start(args=<unavailable>) at qemu-thread-posix.c:541:9
>     frame #19: 0x8b223034 libsystem_pthread.dylib`_pthread_start + 136

Can't see QList or QObject in this backtrace.  object_unref() is QOM,
not QObject.

> However when running 'make check-unit', qobject_is_equal_list_test()
> is successful, so I'm a bit confused...
>
>>           static const int gpad_keycode[5] = { 0xc8, 0xd0, 0xcb, 0xcd, 0x1d };
>> +        DeviceState *gpad;
>>   -        gpad_irq[0] = qemu_irq_invert(gpio_in[GPIO_E][0]); /* up */
>> -        gpad_irq[1] = qemu_irq_invert(gpio_in[GPIO_E][1]); /* down */
>> -        gpad_irq[2] = qemu_irq_invert(gpio_in[GPIO_E][2]); /* left */
>> -        gpad_irq[3] = qemu_irq_invert(gpio_in[GPIO_E][3]); /* right */
>> -        gpad_irq[4] = qemu_irq_invert(gpio_in[GPIO_F][1]); /* select */
>> +        gpad = qdev_new(TYPE_STELLARIS_GAMEPAD);
>> +        for (i = 0; i < ARRAY_SIZE(gpad_keycode); i++) {
>> +            qlist_append_int(gpad_keycode_list, gpad_keycode[i]);
>> +        }
>> +        qdev_prop_set_array(gpad, "keycodes", gpad_keycode_list);
>> +        sysbus_realize_and_unref(SYS_BUS_DEVICE(gpad), &error_fatal);
>>   -        stellaris_gamepad_init(5, gpad_irq, gpad_keycode);
>> +        qdev_connect_gpio_out(gpad, 0,
>> +                              qemu_irq_invert(gpio_in[GPIO_E][0])); /* up */
>> +        qdev_connect_gpio_out(gpad, 1,
>> +                              qemu_irq_invert(gpio_in[GPIO_E][1])); /* down */
>> +        qdev_connect_gpio_out(gpad, 2,
>> +                              qemu_irq_invert(gpio_in[GPIO_E][2])); /* left */
>> +        qdev_connect_gpio_out(gpad, 3,
>> +                              qemu_irq_invert(gpio_in[GPIO_E][3])); /* right */
>> +        qdev_connect_gpio_out(gpad, 4,
>> +                              qemu_irq_invert(gpio_in[GPIO_F][1])); /* select */
>>       }

On 30/10/23 15:25, Markus Armbruster wrote:
> Philippe Mathieu-Daudé <philmd@linaro.org> writes:
> 
>> Hi Peter,
>>
>> Cc'ing Markus for QObject.
>>
>> On 30/10/23 12:48, Peter Maydell wrote:
>>> Convert the hw/input/stellaris_input device to qdev.
>>> The interface uses an array property for the board to specify the
>>> keycodes to use, so the s->keycodes memory is now allocated by the
>>> array-property machinery.
>>> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
>>> Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>>> Tested-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>>> ---
>>> v1->v2: drop private/public comment lines
>>> ---
>>>    include/hw/input/stellaris_gamepad.h | 22 ++++++++-
>>>    hw/arm/stellaris.c                   | 26 +++++++---
>>>    hw/input/stellaris_gamepad.c         | 73 +++++++++++++++++++---------
>>>    3 files changed, 89 insertions(+), 32 deletions(-)
>>
>>
>>> diff --git a/hw/arm/stellaris.c b/hw/arm/stellaris.c
>>> index 96585dd7106..707b0dae375 100644
>>> --- a/hw/arm/stellaris.c
>>> +++ b/hw/arm/stellaris.c
>>> @@ -31,6 +31,7 @@
>>>    #include "hw/timer/stellaris-gptm.h"
>>>    #include "hw/qdev-clock.h"
>>>    #include "qom/object.h"
>>> +#include "qapi/qmp/qlist.h"
>>>      #define GPIO_A 0
>>>    #define GPIO_B 1
>>> @@ -1274,16 +1275,27 @@ static void stellaris_init(MachineState *ms, stellaris_board_info *board)
>>>            sysbus_connect_irq(SYS_BUS_DEVICE(enet), 0, qdev_get_gpio_in(nvic, 42));
>>>        }
>>>        if (board->peripherals & BP_GAMEPAD) {
>>> -        qemu_irq gpad_irq[5];
>>> +        QList *gpad_keycode_list = qlist_new();
>>
>> I'm trying to understand better qlist_new(), but unfortunately there
>> is not much documentation. Looking at how the allocated list was
>> released, I found use of g_autoptr in tests/unit/check-qobject.c,
>> so I tried:
>>
>>             g_autoptr(QList) gpad_keycode_list = qlist_new();
> 
> QObject and its subtypes QDict, QList, QString, ... are reference
> counted.  qFOO_new() ist to be paired with qFOO_unref() or
> qobject_unref().
> 
> Your use of g_autoptr(QList) should work.

Peter figured qdev_prop_set_array() takes the ownership, so using
g_autoptr triggers a double-free:

https://lore.kernel.org/qemu-devel/CAFEAcA_GC8ypM2Y94KCU9Q_dntF6Na+igu-+0JZJ+MvPFE_HcA@mail.gmail.com/