[libvirt] [PATCH] virBitmapFree: Change the function to a macro

[Liuji (Jeremy)]

> -----Original Message-----
> From: Daniel P. Berrange [mailto:berrange at redhat.com]
> Sent: Friday, September 06, 2013 6:37 PM
> To: Liuji (Jeremy)
> Cc: libvir-list at redhat.com; Jinbo (Justin); Luohao (brian); Haofeng
> Subject: Re: [libvirt] [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.