Re: [Qemu-devel] [PATCH v4 5/8] qom: add object_new_with_props / object_new_withpropv constructors

[Andreas Färber]

Am 19.05.2015 um 17:55 schrieb Daniel P. Berrange:
> On Tue, May 19, 2015 at 05:52:14PM +0200, Andreas Färber wrote:
>> Am 13.05.2015 um 18:14 schrieb Daniel P. Berrange:
>>> +Object *object_new_with_propv(const char *typename,
>>> +                              Object *parent,
>>> +                              const char *id,
>>> +                              Error **errp,
>>> +                              va_list vargs)
>>> +{
>>> +    Object *obj;
>>> +    ObjectClass *klass;
>>> +    Error *local_err = NULL;
>>> +
>>> +    klass = object_class_by_name(typename);
>>> +    if (!klass) {
>>> +        error_setg(errp, "invalid object type: %s", typename);
>>> +        return NULL;
>>> +    }
>>> +
>>> +    if (object_class_is_abstract(klass)) {
>>> +        error_setg(errp, "object type '%s' is abstract", typename);
>>> +        return NULL;
>>> +    }
>>> +    obj = object_new(typename);
>>> +
>>> +    if (object_set_propv(obj, &local_err, vargs) < 0) {
>>> +        goto error;
>>> +    }
>>> +
>>> +    object_property_add_child(parent, id, obj, &local_err);
>>> +    if (local_err) {
>>> +        goto error;
>>> +    }
>>> +
>>> +    if (object_dynamic_cast(obj, TYPE_USER_CREATABLE)) {
>>> +        user_creatable_complete(obj, &local_err);
>>> +        if (local_err) {
>>> +            object_unparent(obj);
>>> +            goto error;
>>> +        }
>>> +    }
>>> +
>>> +    object_unref(OBJECT(obj));
>>> +    return obj;
>>
>> This looks fishy. Either we return obj and need to retain a ref for that
>> (caller's responsibility to unref) or we unref it and return void.
>> Or am I misreading the code?
> 
> Paolo told me on previous posting that object_property_add_child()
> holds a reference on 'obj' for as long as it is registered in the
> object hierarchy composition. So it sufficient to rely on that long
> term reference, and let the caller dispose of the object by calling
> object_unparent(obj) when finally done.

Heh, that's why I like reviewing my patches myself. That reference can
go away via QMP. For the CPU I have a patch pending to fix some corner
cases. We can do that as follow-up here though.

Regards,
Andreas

-- 
SUSE Linux GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Felix Imendörffer, Jane Smithard, Dilip Upmanyu, Graham Norton; HRB
21284 (AG Nürnberg)