On 4/28/20 11:34 AM, Markus Armbruster wrote:
Signed-off-by: Markus Armbruster <arm...@redhat.com> --- qom/object.c | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-)diff --git a/qom/object.c b/qom/object.c index 1812f79224..e06c78f9a5 100644 --- a/qom/object.c +++ b/qom/object.c @@ -571,18 +571,18 @@ void object_initialize_childv(Object *parentobj, const char *propname, } }+out:/* - * Since object_property_add_child added a reference to the child object, - * we can drop the reference added by object_initialize(), so the child - * property will own the only reference to the object. + * We want @obj's reference to be 1 on success, 0 on failure. + * On success, it's 2: one taken by object_initialize(), and one + * by object_property_add_child(). + * On failure in object_initialize() or earlier, it's 1. + * On failure afterwards, it's also 1: object_unparent() releases + * the reference taken by object_property_add_child().
Useful comment.
*/ object_unref(obj);-out:- if (local_err) { - error_propagate(errp, local_err); - object_unref(obj); - } + error_propagate(errp, local_err);
But looking at just the code (even without comments), this is safe - pre-patch, the only way to get to the out: label without a 'goto' is on the success path, at which point we can deduce that object_unref() is called exactly once whether on the success path or on the local_err path, at which point moving the label up and making the object_unref() unconditional is the same semantics. With that hoisted, error_propagate is the only thing remaining under an 'if (local_err)' guard, and it is safe to call that when there is no error. No semantic change, at which point all that remains to review is the accuracy of the comment.
Reviewed-by: Eric Blake <ebl...@redhat.com> -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3226 Virtualization: qemu.org | libvirt.org
