On 15:23 Fri 23 Oct , Marc-André Lureau wrote: > 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. Yes, Peter encountered a double-free error because of this missing object_ref when he applied my Raspberry PI CPRMAN series, which makes use of qdev_alias_clock.
Peter: do you consider taking this patch to be able to pick up the CPRMAN series again? > > It would have been nice to have some unit tests for qdev-clock.h.. Yes I agree. -- Luc > > 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 --