Hi

On Tue, Oct 20, 2020 at 1:11 PM Luc Michel <l...@lmichel.fr> wrote:

> When aliasing a clock with the qdev_alias_clock() function, a new link
> property is created on the device aliasing the clock. The link points
> to the aliased clock and use the OBJ_PROP_LINK_STRONG flag. This
> property is read only since it does not provide a check callback for
> modifications.
>
> The object_property_add_link() documentation stats that with
> OBJ_PROP_LINK_STRONG properties, the linked object reference count get
> decremented when the property is deleted. But it is _not_ incremented on
> creation (object_property_add_link() does not actually know the link).
>
> This commit increments the reference count on the aliased clock to
> ensure the aliased clock stays alive during the property lifetime, and
> to avoid a double-free memory error when the property gets deleted.
>
> Reviewed-by: Philippe Mathieu-Daudé <f4...@amsat.org>
> Signed-off-by: Luc Michel <l...@lmichel.fr>
> ---
>

In principle, that makes sense. But I don't see any users of that API yet.

It would have been nice to have some unit tests for qdev-clock.h..

 hw/core/qdev-clock.c | 8 ++++++++
>  1 file changed, 8 insertions(+)
>
> diff --git a/hw/core/qdev-clock.c b/hw/core/qdev-clock.c
> index 6a9a340d0f..eb05f2a13c 100644
> --- a/hw/core/qdev-clock.c
> +++ b/hw/core/qdev-clock.c
> @@ -59,10 +59,18 @@ static NamedClockList *qdev_init_clocklist(DeviceState
> *dev, const char *name,
>      } else {
>          object_property_add_link(OBJECT(dev), name,
>                                   object_get_typename(OBJECT(clk)),
>                                   (Object **) &ncl->clock,
>                                   NULL, OBJ_PROP_LINK_STRONG);
> +        /*
> +         * Since the link property has the OBJ_PROP_LINK_STRONG flag, the
> clk
> +         * object reference count gets decremented on property deletion.
> +         * However object_property_add_link does not increment it since it
> +         * doesn't know the linked object. Increment it here to ensure the
> +         * aliased clock stays alive during this device life-time.
> +         */
> +        object_ref(OBJECT(clk));
>      }
>
>      ncl->clock = clk;
>
>      QLIST_INSERT_HEAD(&dev->clocks, ncl, node);
> --
> 2.28.0
>
>
>

-- 
Marc-André Lureau

Reply via email to