Hi Chandan

On Mon, Dec 22, 2025 at 9:56 AM Chandan Somani <[email protected]> wrote:
>
> Before this patch, users of the property array would free the
> array themselves in their cleanup functions. This causes
> inconsistencies where some users leak the array and some free them.
>
> This patch makes it so that the property array's release function
> frees the property array (instead of just its elements). It fixes any
> leaks and requires less code.
>
> Signed-off-by: Chandan Somani <[email protected]>

I sent a similar series earlier and forgot about it:
https://patchew.org/QEMU/[email protected]/

Maybe you could combine the two?

> ---
>  block/accounting.c                |  1 -
>  hw/core/qdev-properties.c         | 20 ++++++++++----------
>  hw/input/stellaris_gamepad.c      |  8 --------
>  hw/intc/arm_gicv3_common.c        |  8 --------
>  hw/intc/rx_icu.c                  |  8 --------
>  hw/misc/arm_sysctl.c              |  2 --
>  hw/misc/mps2-scc.c                |  8 --------
>  hw/net/rocker/rocker.c            |  1 -
>  hw/nvram/xlnx-efuse.c             |  8 --------
>  hw/nvram/xlnx-versal-efuse-ctrl.c |  8 --------
>  10 files changed, 10 insertions(+), 62 deletions(-)
>
> diff --git a/block/accounting.c b/block/accounting.c
> index 0933c61f3a..5cf51f029b 100644
> --- a/block/accounting.c
> +++ b/block/accounting.c
> @@ -73,7 +73,6 @@ bool block_acct_setup(BlockAcctStats *stats, enum OnOffAuto 
> account_invalid,
>              }
>              block_acct_add_interval(stats, stats_intervals[i]);
>          }
> -        g_free(stats_intervals);
>      }
>      return true;
>  }
> diff --git a/hw/core/qdev-properties.c b/hw/core/qdev-properties.c
> index 0930d64252..455c28535a 100644
> --- a/hw/core/qdev-properties.c
> +++ b/hw/core/qdev-properties.c
> @@ -673,10 +673,8 @@ static Property array_elem_prop(Object *obj, const 
> Property *parent_prop,
>
>  /*
>   * Object property release callback for array properties: We call the
> - * underlying element's property release hook for each element.
> - *
> - * Note that it is the responsibility of the individual device's deinit
> - * to free the array proper.
> + * underlying element's property release hook for each element and free the
> + * property array.
>   */
>  static void release_prop_array(Object *obj, const char *name, void *opaque)
>  {
> @@ -686,14 +684,16 @@ static void release_prop_array(Object *obj, const char 
> *name, void *opaque)
>      char *elem = *arrayptr;
>      int i;
>
> -    if (!prop->arrayinfo->release) {
> -        return;
> +    if (prop->arrayinfo->release) {
> +        for (i = 0; i < *alenptr; i++) {
> +            Property elem_prop = array_elem_prop(obj, prop, name, elem);
> +            prop->arrayinfo->release(obj, NULL, &elem_prop);
> +            elem += prop->arrayfieldsize;
> +        }
>      }
>
> -    for (i = 0; i < *alenptr; i++) {
> -        Property elem_prop = array_elem_prop(obj, prop, name, elem);
> -        prop->arrayinfo->release(obj, NULL, &elem_prop);
> -        elem += prop->arrayfieldsize;
> +    if (*arrayptr) {
> +        g_free(*arrayptr);
>      }
>  }
>
> diff --git a/hw/input/stellaris_gamepad.c b/hw/input/stellaris_gamepad.c
> index fec1161c9c..207064dacb 100644
> --- a/hw/input/stellaris_gamepad.c
> +++ b/hw/input/stellaris_gamepad.c
> @@ -63,13 +63,6 @@ static void stellaris_gamepad_realize(DeviceState *dev, 
> Error **errp)
>      qemu_input_handler_register(dev, &stellaris_gamepad_handler);
>  }
>
> -static void stellaris_gamepad_finalize(Object *obj)
> -{
> -    StellarisGamepad *s = STELLARIS_GAMEPAD(obj);
> -
> -    g_free(s->keycodes);
> -}
> -
>  static void stellaris_gamepad_reset_enter(Object *obj, ResetType type)
>  {
>      StellarisGamepad *s = STELLARIS_GAMEPAD(obj);
> @@ -98,7 +91,6 @@ static const TypeInfo stellaris_gamepad_info[] = {
>          .name = TYPE_STELLARIS_GAMEPAD,
>          .parent = TYPE_SYS_BUS_DEVICE,
>          .instance_size = sizeof(StellarisGamepad),
> -        .instance_finalize = stellaris_gamepad_finalize,
>          .class_init = stellaris_gamepad_class_init,
>      },
>  };
> diff --git a/hw/intc/arm_gicv3_common.c b/hw/intc/arm_gicv3_common.c
> index 2d0df6da86..26d694d2ab 100644
> --- a/hw/intc/arm_gicv3_common.c
> +++ b/hw/intc/arm_gicv3_common.c
> @@ -488,13 +488,6 @@ static void arm_gicv3_common_realize(DeviceState *dev, 
> Error **errp)
>      s->itslist = g_ptr_array_new();
>  }
>
> -static void arm_gicv3_finalize(Object *obj)
> -{
> -    GICv3State *s = ARM_GICV3_COMMON(obj);
> -
> -    g_free(s->redist_region_count);
> -}
> -
>  static void arm_gicv3_common_reset_hold(Object *obj, ResetType type)
>  {
>      GICv3State *s = ARM_GICV3_COMMON(obj);
> @@ -644,7 +637,6 @@ static const TypeInfo arm_gicv3_common_type = {
>      .instance_size = sizeof(GICv3State),
>      .class_size = sizeof(ARMGICv3CommonClass),
>      .class_init = arm_gicv3_common_class_init,
> -    .instance_finalize = arm_gicv3_finalize,
>      .abstract = true,
>      .interfaces = (const InterfaceInfo[]) {
>          { TYPE_ARM_LINUX_BOOT_IF },
> diff --git a/hw/intc/rx_icu.c b/hw/intc/rx_icu.c
> index f8615527b7..85da0624f6 100644
> --- a/hw/intc/rx_icu.c
> +++ b/hw/intc/rx_icu.c
> @@ -334,13 +334,6 @@ static void rxicu_init(Object *obj)
>      sysbus_init_irq(d, &icu->_swi);
>  }
>
> -static void rxicu_fini(Object *obj)
> -{
> -    RXICUState *icu = RX_ICU(obj);
> -    g_free(icu->map);
> -    g_free(icu->init_sense);
> -}
> -
>  static const VMStateDescription vmstate_rxicu = {
>      .name = "rx-icu",
>      .version_id = 1,
> @@ -382,7 +375,6 @@ static const TypeInfo rxicu_info = {
>      .parent = TYPE_SYS_BUS_DEVICE,
>      .instance_size = sizeof(RXICUState),
>      .instance_init = rxicu_init,
> -    .instance_finalize = rxicu_fini,
>      .class_init = rxicu_class_init,
>  };
>
> diff --git a/hw/misc/arm_sysctl.c b/hw/misc/arm_sysctl.c
> index 0f4e37cd47..e715ff9475 100644
> --- a/hw/misc/arm_sysctl.c
> +++ b/hw/misc/arm_sysctl.c
> @@ -618,9 +618,7 @@ static void arm_sysctl_finalize(Object *obj)
>  {
>      arm_sysctl_state *s = ARM_SYSCTL(obj);
>
> -    g_free(s->db_voltage);
>      g_free(s->db_clock);
> -    g_free(s->db_clock_reset);
>  }
>
>  static const Property arm_sysctl_properties[] = {
> diff --git a/hw/misc/mps2-scc.c b/hw/misc/mps2-scc.c
> index a9a5d4a535..acb0f5773b 100644
> --- a/hw/misc/mps2-scc.c
> +++ b/hw/misc/mps2-scc.c
> @@ -405,13 +405,6 @@ static void mps2_scc_realize(DeviceState *dev, Error 
> **errp)
>      s->oscclk = g_new0(uint32_t, s->num_oscclk);
>  }
>
> -static void mps2_scc_finalize(Object *obj)
> -{
> -    MPS2SCC *s = MPS2_SCC(obj);
> -
> -    g_free(s->oscclk_reset);
> -}
> -
>  static bool cfg7_needed(void *opaque)
>  {
>      MPS2SCC *s = opaque;
> @@ -489,7 +482,6 @@ static const TypeInfo mps2_scc_info = {
>      .parent = TYPE_SYS_BUS_DEVICE,
>      .instance_size = sizeof(MPS2SCC),
>      .instance_init = mps2_scc_init,
> -    .instance_finalize = mps2_scc_finalize,
>      .class_init = mps2_scc_class_init,
>  };
>
> diff --git a/hw/net/rocker/rocker.c b/hw/net/rocker/rocker.c
> index cc49701dd3..cbc7bd3ed9 100644
> --- a/hw/net/rocker/rocker.c
> +++ b/hw/net/rocker/rocker.c
> @@ -1429,7 +1429,6 @@ static void pci_rocker_uninit(PCIDevice *dev)
>              world_free(r->worlds[i]);
>          }
>      }
> -    g_free(r->fp_ports_peers);
>  }
>
>  static void rocker_reset(DeviceState *dev)
> diff --git a/hw/nvram/xlnx-efuse.c b/hw/nvram/xlnx-efuse.c
> index 4c23f8b931..1875fdb953 100644
> --- a/hw/nvram/xlnx-efuse.c
> +++ b/hw/nvram/xlnx-efuse.c
> @@ -224,13 +224,6 @@ static void efuse_realize(DeviceState *dev, Error **errp)
>      }
>  }
>
> -static void efuse_finalize(Object *obj)
> -{
> -    XlnxEFuse *s = XLNX_EFUSE(obj);
> -
> -    g_free(s->ro_bits);
> -}
> -
>  static void efuse_prop_set_drive(Object *obj, Visitor *v, const char *name,
>                                   void *opaque, Error **errp)
>  {
> @@ -288,7 +281,6 @@ static const TypeInfo efuse_info = {
>      .name          = TYPE_XLNX_EFUSE,
>      .parent        = TYPE_DEVICE,
>      .instance_size = sizeof(XlnxEFuse),
> -    .instance_finalize = efuse_finalize,
>      .class_init    = efuse_class_init,
>  };
>
> diff --git a/hw/nvram/xlnx-versal-efuse-ctrl.c 
> b/hw/nvram/xlnx-versal-efuse-ctrl.c
> index 6f17f32a0c..4d0c2c8404 100644
> --- a/hw/nvram/xlnx-versal-efuse-ctrl.c
> +++ b/hw/nvram/xlnx-versal-efuse-ctrl.c
> @@ -724,13 +724,6 @@ static void efuse_ctrl_init(Object *obj)
>      sysbus_init_irq(sbd, &s->irq_efuse_imr);
>  }
>
> -static void efuse_ctrl_finalize(Object *obj)
> -{
> -    XlnxVersalEFuseCtrl *s = XLNX_VERSAL_EFUSE_CTRL(obj);
> -
> -    g_free(s->extra_pg0_lock_spec);
> -}
> -
>  static const VMStateDescription vmstate_efuse_ctrl = {
>      .name = TYPE_XLNX_VERSAL_EFUSE_CTRL,
>      .version_id = 1,
> @@ -767,7 +760,6 @@ static const TypeInfo efuse_ctrl_info = {
>      .instance_size = sizeof(XlnxVersalEFuseCtrl),
>      .class_init    = efuse_ctrl_class_init,
>      .instance_init = efuse_ctrl_init,
> -    .instance_finalize = efuse_ctrl_finalize,
>  };
>
>  static void efuse_ctrl_register_types(void)
> --
> 2.51.1
>
>


-- 
Marc-André Lureau

Reply via email to