Re: [Qemu-devel] [PATCH] hw/arm/virt: fix cpu object reference leak

[Peter Maydell]

On 16 February 2017 at 15:11, Igor Mammedov <address@hidden> wrote:
> On Thu, 16 Feb 2017 14:18:05 +0000
> Peter Maydell <address@hidden> wrote:
>> I've always found the object reference semantics somewhat
>> confusing (why does realizing a device add a reference,
>> for instance?). Do we document them anywhere?
> I'm not aware of a place where it's documented.
>
> currently device_realize() sets parent thus increasing
> ref counter only if device creator haven't set parent
> explicitly.

It doesn't seem to:

static void device_realize(DeviceState *dev, Error **errp)
{
    DeviceClass *dc = DEVICE_GET_CLASS(dev);

    if (dc->init) {
        int rc = dc->init(dev);
        if (rc < 0) {
            error_setg(errp, "Device initialization failed.");
            return;
        }
    }
}

...it just calls the device's init function if it has one.

It's also pretty confusing that qdev_try_create()
and qdev_create() return a pointer to an object
that has been put into a bus and had unref called
(so the caller doesn't need to manually unref),
but plain object_new() returns a pointer to an
object that hasn't been put into a bus, yet
realizing does put it into a bus but doesn't do the
corresponding unref.

I'd be a lot happier if we had clear documentation that
described how our object model, plugging things into buses,
etc handled reference counting and what the expected
"correct" code patterns are for using it.

That said, I guess this patch is correct so I'm applying
it to target-arm.next.

thanks
-- PMM