Re: [PATCH] hw/core/qdev-clock: add a reference on aliased clocks

[Philippe Mathieu-Daudé]

On 10/20/20 9:58 AM, Philippe Mathieu-Daudé wrote:
> Cc'ing Markus/Marc-André too.
>
> On 10/20/20 9:44 AM, Luc Michel 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 get deleted.
> >
> > Signed-off-by: Luc Michel <luc@lmichel.fr>
> > ---
> >   hw/core/qdev-clock.c | 1 +
> >   1 file changed, 1 insertion(+)
> >
> > diff --git a/hw/core/qdev-clock.c b/hw/core/qdev-clock.c
> > index 6a9a340d0f..5f5e143702 100644
> > --- a/hw/core/qdev-clock.c
> > +++ b/hw/core/qdev-clock.c
> > @@ -59,10 +59,11 @@ 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);
> > +        object_ref(OBJECT(clk));

OK, this is particular to this model because device_finalize()
garbage-collector and calls qdev_finalize_clocklist().

With a comment explaining why we need this call:
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>

> >       }
> >       ncl->clock = clk;
> >       QLIST_INSERT_HEAD(&dev->clocks, ncl, node);