From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([140.186.70.92]:56721) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1S0Foo-0006GL-W2 for qemu-devel@nongnu.org; Wed, 22 Feb 2012 12:17:35 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1S0Foj-0000SU-0e for qemu-devel@nongnu.org; Wed, 22 Feb 2012 12:17:30 -0500 Received: from mx1.redhat.com ([209.132.183.28]:49867) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1S0Foi-0000SN-Hr for qemu-devel@nongnu.org; Wed, 22 Feb 2012 12:17:24 -0500 Message-ID: <4F45231E.2030600@redhat.com> Date: Wed, 22 Feb 2012 18:17:18 +0100 From: Paolo Bonzini MIME-Version: 1.0 References: <1329930831-26837-1-git-send-email-alexander_barabash@mentor.com> In-Reply-To: <1329930831-26837-1-git-send-email-alexander_barabash@mentor.com> Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH] qom: In function object_set_link_property(), first call object_ref(), then object_unref(). List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: alexander_barabash@mentor.com Cc: qemu-devel@nongnu.org On 02/22/2012 06:13 PM, alexander_barabash@mentor.com wrote: > From: Alexander Barabash > > In the old implementation, if the new value of the property links > to the same object, as the old value, that object is first unref-ed, > and then ref-ed. This leads to unintended deinitialization of that object. > > In the new implementation, this is fixed. > > Signed-off-by: Alexander Barabash > --- > qom/object.c | 18 +++++++++++++----- > 1 files changed, 13 insertions(+), 5 deletions(-) > > diff --git a/qom/object.c b/qom/object.c > index 941c291..d1b3ac7 100644 > --- a/qom/object.c > +++ b/qom/object.c > @@ -892,19 +892,19 @@ static void object_set_link_property(Object *obj, Visitor *v, void *opaque, > const char *name, Error **errp) > { > Object **child = opaque; > + Object *old_target; > bool ambiguous = false; > const char *type; > char *path; > gchar *target_type; > + bool clear_old_target = true; > > type = object_property_get_type(obj, name, NULL); > > visit_type_str(v, &path, name, errp); > > - if (*child) { > - object_unref(*child); > - *child = NULL; > - } > + old_target = *child; > + *child = NULL; You can just remove the unref here... > if (strcmp(path, "") != 0) { > Object *target; > @@ -916,7 +916,11 @@ static void object_set_link_property(Object *obj, Visitor *v, void *opaque, > if (ambiguous) { > error_set(errp, QERR_AMBIGUOUS_PATH, path); > } else if (target) { > - object_ref(target); > + if (target != old_target) { > + object_ref(target); ... leave the unconditional ref to target here... > + } else { > + clear_old_target = false; > + } > *child = target; > } else { > target = object_resolve_path(path, &ambiguous); > @@ -930,6 +934,10 @@ static void object_set_link_property(Object *obj, Visitor *v, void *opaque, > } > > g_free(path); > + > + if (clear_old_target && (old_target != NULL)) { > + object_unref(old_target); ... and leave this unref on old_target, without the need for clear_old_target. > + } > } > > void object_property_add_link(Object *obj, const char *name, Paolo