Re: [PATCH] qdev: Free property array on release

[Stefan Hajnoczi <stefanha@redhat.com>]

[-- Attachment #1: Type: text/plain, Size: 2255 bytes --]

On Sun, Dec 21, 2025 at 09:50:05PM -0800, Chandan Somani 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 <csomani@redhat.com>
> ---
>  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(-)

Missing hw/virtio/virtio-iommu-pci.c?

The following DEFINE_PROP_ARRAY() users unintentionally leak their
arrays and are fixed by this patch (I think it's worth mentioning this
in the commit description):
- hw/display/apple-gfx-mmio.m
- hw/display/apple-gfx-pci.m
- hw/net/virtio-net.c
- hw/riscv/riscv_hart.c

> @@ -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);

It is safe to call g_free(NULL), so if (*arrayptr) is unnecessary.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]