> -----Original Message----- > From: Daniel P. Berrange [mailto:berrange@xxxxxxxxxx] > Sent: Friday, September 06, 2013 6:37 PM > To: Liuji (Jeremy) > Cc: libvir-list@xxxxxxxxxx; Jinbo (Justin); Luohao (brian); Haofeng > Subject: Re: [PATCH] virBitmapFree: Change the function to a macro > > On Fri, Sep 06, 2013 at 10:30:56AM +0000, Liuji (Jeremy) wrote: > > The parameter of virBitmapFree function is just a pointer, not a pointer of > pointer. > > The second VIR_FREE on virBitmapFree only assign NULL to the formal > parameter. > > After calling the virBitmapFree function, the actual parameter are still not > NULL. > > There are many code segment don't assign NULL to the formal parameter > after calling > > the virBitmapFree function. This will bring potential risks. > > > > A problem scenario: > > 1) The XML of VM contain the below segment: > > <numatune> > > <memory mode='preferred' placement='auto' nodeset='0'/> > > </numatune> > > 2)virsh create the VM > > 3)In the virDomainDefParseXML funtion: > > /* Ignore 'nodeset' if 'placement' is 'auto' finally */ > > if (placement_mode == > VIR_NUMA_TUNE_MEM_PLACEMENT_MODE_AUTO) { > > > virBitmapFree(def->numatune.memory.nodemask); > > def->numatune.memory.nodemask = NULL; > > } > > 4)Then, virsh destroy the VM. In the virDomainDefFree funtion, it also call the > > virBitmapFree function to free the nodemask: > > virBitmapFree(def->numatune.memory.nodemask); > > But after this call, the value of def->numatune.memory.nodemask is still not > NULL. > > This will generate an exception. > > Have you got an actual crash happening today, or is this just a theoretical > problem you're trying to address ? Yes，it's an actual crash problem. I found the problem in the above problem scenario in my first mail. But, my first mail has a mistake: The description of step 3 and 4 of "A problem scenario:" should be like this: 3)In the virDomainDefParseXML funtion: /* Ignore 'nodeset' if 'placement' is 'auto' finally */ if (placement_mode == VIR_NUMA_TUNE_MEM_PLACEMENT_MODE_AUTO) virBitmapFree(def->numatune.memory.nodemask); 4)Then, virsh destroy the VM. In the virDomainDefFree funtion, it also call the virBitmapFree funtion to free the nodemask: virBitmapFree(def->numatune.memory.nodemask); But before this call, the value of def->numatune.memory.nodemask is still not NULL, and the address which is pointed by the pointer has been freed. The VIR_FREE on virBitmapFree function will free the address again. -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list