Il 11/03/2013 13:04, Cornelia Huck ha scritto: > On Fri, 8 Mar 2013 21:11:13 +0100 > Alexander Graf <ag...@suse.de> wrote: > >> >> On 25.02.2013, at 12:10, Christian Borntraeger wrote: >> >>> On 25/02/13 11:44, Paolo Bonzini wrote: >>>> Il 25/02/2013 09:09, Christian Borntraeger ha scritto: >>>>> Hmm, the old sequence was >>>>> >>>>> object_unparent(OBJECT(dev)); >>>>> qdev_free(dev) ---+ >>>>> | >>>>> V >>>>> ... >>>>> object_unparent(OBJECT(dev)); now the last reference is gone, >>>>> object is freed >>>>> object_unref(OBJECT(dev)); now the reference of a deleted >>>>> object becomes -1 >>>>> ... >>>>> >>>>> Isnt that a problem in itself that we modify a reference counter in an >>>>> deleted object? >>>> >>>> The second object_unparent should do nothing. So before you had: >>>> >>>> object_unparent(OBJECT(dev)); leaves refcount=1 >>>> qdev_free(dev) ---+ >>>> | >>>> V >>>> object_unparent(OBJECT(dev)); do nothing >>>> object_unref(OBJECT(dev)); refcount=0, object freed >>>> >>>> After the object_unref was removed you had: >>>> >>>> object_unparent(OBJECT(dev)); refcount=0, object freed >>>> qdev_free(dev) ---+ >>>> | >>>> V >>>> object_unparent(OBJECT(dev)); dangling pointer! >>>> >>> >>> >>> Got it. Thanks >> >> So is the patch valid? > > To my understanding, yes.
Yes, except that the "fixed a crash" part in the commit message is probably no longer accurate. No big deal. :) Paolo
