On Mon, 11 Mar 2013 13:22:45 +0100 Alexander Graf <ag...@suse.de> wrote:
> On 03/11/2013 01:16 PM, Paolo Bonzini wrote: > > 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. :) > > Ok, Connie could you please include it in your next pull then please? Sure, will do. > > > Alex >